Return Styles: Pseud0ch, Terminal, Valhalla, NES, Geocities, Blue Moon. Entire thread

Happy Numbers

Name: Tynach 2009-09-03 19:34

Hey, I just found this board on 4chan (didn't know about it before), and I wanted to know what you guys thought of my program that checks to see if a number is happy. It was originally written in Python, but as a means to learn C, I re-wrote it in C a while back. Recently, I decided I wanted to find out if I really understood pointers as well as I thought, so I re-wrote the C version from scratch, though implemented some of the same code (though heavily modified to work with my new format) to use pointers so it uses less RAM.

Here's the source:

#include <stdio.h>

void check(char *);
char checkh(long long int, long long int *);
void usragain(char *, char *);
void getnum(long long int *);
void flushin();

int main()
{
    char run = 1;
    char state = 0;
    printf("Happy Number Checker v 2.0\n\n");
    while(run == 1) {
        switch (state) {
        case 0:
            check(&state);
            break;
        case 1:
            usragain(&run, &state);
            break;
        }
    }
    return 0;
}

void check(char *ret)
{
    long long int orig;
    getnum(&orig);
    *ret = checkh(orig, &orig);
    if (*ret == 0) {
        printf("%lld is not happy. Try again.\n\n", orig);
    } else if (*ret == 1) {
        printf("%lld is happy!\n\n", orig);
    }
}

char checkh(long long int guess, long long int *orig)
{
    long long int tmp = guess;
    long long int sum;
    char digit;
    char loop = 1;
   
    while (loop == 1) {
        sum = 0;
        while (tmp != 0) {
            digit = tmp % 10;
            sum += digit * digit;
            tmp /= 10;
        }
        if (sum == 1) {
            loop = 0;
            return 1;http://www.qso.com/carlautta/colin/Beta/happy.c
        } else if (sum == 42 || sum == *orig) {
            loop = 0;
            return 0;
        } else {
            loop = 1;
            tmp = sum;
        }
    }
}

void usragain(char *yn, char *state)
{
    *yn = 2;
    printf("Would you like to check another number (y/n)? ");
    while (*yn == 2) {
        flushin();
        scanf("%c", yn);
        printf("\n");
        if (*yn == 'y') {
            *yn = 1;
            *state = 0;
        } else if (*yn == 'n') {
            *yn = 0;
        } else {
            printf("Invalid input, type y or n.\n");
            *yn = 2;
        }
    }
}

void getnum(long long int *guess)
{
    int test = 0;
    while (test == 0) {
        printf("Please insert a number: ");
        test = scanf("%lld", guess);
        if (test == 0) {
            flushin();
            printf("\n\nInvalid input. Try again.\n\n");
        }
    }
}

void flushin()
{
    int ch = 0;
    while ((ch = getc(stdin)) != EOF && ch != '\n') {
        continue;
    }
}

I'm a newb to text boards here on 4chan, so I may be missing a way to insert code properly (in some forums it's and). Please tell me if I fail at this and should become an hero.

Name: Anonymous 2009-09-03 23:11

1. You should give your functions names that will tell the reader what the function does without having to analyse the code.  For instance, what does check() check?  Or checkh() for that matter?  Why are there two?  It may make sense if you wrote the program, but if you come back to this code in a couple months it may be confusing, and certainly if another person wanted to work on your code they'd have to have a good look at both functions to understand what they mean.  As it stands, only getnum() and flushin() tell me exactly what it does just by the name of the function.

2. Instead of using long long int, include stdint.h and use uint64_t as the datatype.  This will portably give you an unsigned 64-bit integer, and is also much less ugly than "unsigned long long int".  (Note: You may need to use the %llu token instead of %llu in your printf if you do this.)

3. Using a char to define the state datatype is unnecessary and misleading.  Use an int, or int8_t instead.

4. In your flushin() function:
   while ((ch = getc(stdin)) != EOF && ch != '\n') {
       continue;
   }
It is unnecessary to use "continue" in the body of the loop.  You can just end the while(); with a semi-colon like that.  You also don't need to store getc(stdin) in a variable.  This function could be re-written as

void flushin()
{
    while ((getc(stdin) != EOF) && (getc(stdin) != '\n'));
}


5. Why does usragain() need to take a char* for the yn argument?  You can just declare char yn local to the function.

6. Again you are performing math with a char in checkh().  Use an integer if you want to store numbers -- if it must be 1 byte, use int8_t or uint8_t as defined in stdint.h.

If you fix this shit then maybe I'll take another look at how you're doing things...  I don't feel like examining every little problem, but for instance, you have a variable defined in the main function (run) that keeps your main while loop running.  However, when the program is terminated, run is not set to 0 in main.  You pass a pointer to it to the usragain function, and that function first stores a character in the pointer and then changes it to either 0 or 1 again.  That's awful design, in my opinion.

Newer Posts
Don't change these.
Name: Email:
Entire Thread Thread List