Made a script on the Luhn's Alg. Unable to understand how to create and call functions clearly for revised script

Question

I am new to programming. I am currently taking online lectures quite rigorously and completed a task using Luhn's Algorithm. It is just one script that runs straight through, but for my future-self I want to learn to code more efficiently as projects get bigger.

That is where my problem comes in. I cannot seem to understand how to define or call functions correctly and unable to revise my script into something more "efficient".

(Everything is already submitted and my script completes the task perfectly, according to the bot, so I am not trying to half-arse my work here just to be clear.)

This is the completed script with only the main function that runs straight through and took me about 12-15 hours to get it working without error from the beginning. Everything is written in C

#include <stdio.h>
#include <cs50.h>
#include <math.h>

int main(void)
{
    // this grabs the number and verifies the correct amount of digits
    int count;
    long number = 0;
    do
    {
        number = get_long("Number: ");
        count = (number == 0) ? 1 : (log10(number) + 1);
        if (count < 13 || count == 14 || count > 16)
        {
            printf("INVALID\n"); // This is to satisfy the uni. bot command check. Need a EOF for numbers with the wrong amount of digits.
            return (0);
        }
    }
    while (count < 13 || count == 14 || count > 16);
    //printf("Digits: %i\n", count);       // test print for debugging

    //This finds the first two digits of input number

    long int two = number;
    while (two >= 100)
    {
        two = two / 10;
    }
    //printf("First two numbers: %li\n", two);      // test print for debugging

    // verifies card using mod10 (Luhn's)
    long sum = 0;
    long bigdigit = 0;
    //printf("\nLUHN Number: %li\n\n", number);     // test print for debugging
    if (count == 13 || count == 15)
    {
        count += 1;
    }
    for (int i = count; i > 0; i--)
    {
        if (i % 2 == 0)
        {
            sum += (number % 10);
        }
        else
        {
            bigdigit = (2 * (number % 10));
            sum += (bigdigit / 10 + bigdigit % 10);
        }
        number = (number / 10);
        //printf("\nI : %i\n", i);      // test print for debugging
        //printf("Sum: %li\n", sum);        // test print for debugging
        //printf("Number: %li\n", number);      // test print for debugging
    }
    if (sum % 10 == 0)
    {
        printf("VALID\n");
    }
    else
    {
        printf("INVALID\n");
        return (0);
    }

    // checks what type of card

    if (two == 34 || two == 37)
    {
        printf("AMEX\n");
        return (0);
    }
    else if (two >= 51 && two <= 55)
    {
        printf("MASTERCARD\n");
        return (0);
    }
    else if (two >= 40 && two <= 49)
    {
        printf("VISA\n");
        return (0);
    }
    else
    {
        printf("INVALID\n");
        return (0);
    }
}

I was trying to split it into 3 functions of main to call.

long input_number();
bool luhn_check();
void company_check();

I am stuck with the second function and not sure if the third should be a void or not.

"Revised" Script v2

#include <stdio.h>
#include <cs50.h>
#include <math.h>

long input_number(long CCN);
int counter(long CCN, int count);
bool luhn_check(long CCN, int count);
long firsttwo(long CCN, long two);
void card_type(long two);


int main()
{
    long CCN = 0;
    int count = 0;
    long two = 0;
    CCN = input_number(CCN);
    count = counter(CCN, count);

    //printf("CCN: %li\n", CCN); //debugging purposes
    //printf("Digits: %i\n", count); //debugging purposes
    luhn_check(CCN, count);
    two = firsttwo(CCN, two);
    //printf("First Two: %li\n", two); //debugging purposes
    card_type(two);
}


    // this grabs the number and verifies the correct amount of digits
long input_number(long CCN)
{
    int count = 0;
    do
    {
        CCN = get_long("Number: ");
        count = (CCN == 0) ? 1 : (log10(CCN) + 1);
        if (count < 13 || count == 14 || count > 16)
        {
            //printf("INVALID\n"); // This is to satisfy the uni. bot command check. Need a EOF
            //return (0);
        }
    }
    while (count < 13 || count == 14 || count > 16);   
    return (CCN);
}

int counter(long CCN, int count)
{
 do
    {
        count = (CCN == 0) ? 1 : (log10(CCN) + 1);
    }
    while (count < 13 || count == 14 || count > 16);   
    return (count);   
}

    // verifies card using mod10 (Luhn's)
bool luhn_check(long CCN, int count)
{
    long sum = 0;
    long bigdigit = 0;
    //printf("\nLUHN Number: %ld\n\n", CCN);     // test print for debugging
    if (count == 13 || count == 15)
    {
        count += 1;
    }
    for (int i = count; i > 0; i--)
    {
        if (i % 2 == 0)
        {
            sum += (CCN % 10);
        }
        else
        {
            bigdigit = (2 * (CCN % 10));
            sum += (bigdigit / 10 + bigdigit % 10);
        }
        CCN = (CCN / 10);
        //printf("\nI : %i\n", i);      // test print for debugging
        //printf("Sum: %li\n", sum);        // test print for debugging
        //printf("Number: %li\n", CCN);      // test print for debugging
    }
    if (sum % 10 == 0)
    {
        printf("VALID\n");
        return (true);
    }
    else
    {
        printf("INVALID\n");
        return (false);
    }
}

            // grabs the first two numbers
long firsttwo(long CCN, long two)
{
    two = CCN;
    //printf("TWO CCN: %li\n", two); // debugging purposes
    while (two >= 100)
    {
        two = two / 10;
    }
    return (two);
}

            // finds card type and ends
void card_type(long two)
{
    if (two == 34 || two == 37)
    {
        printf("AMEX\n");
        //return (0);
    }
    else if (two >= 51 && two <= 55)
    {
        printf("MASTERCARD\n");
        //return (0);
    }
    else if (two >= 40 && two <= 49)
    {
        printf("VISA\n");
        //return (0);
    }
    else
    {
        printf("INVALID\n");
        //return (0);
    }
}

I have completed the second version of the script with your suggestions, bar the str of input, I will try to tackle that method in the next version as I have not handled that type yet.

Besides running the program with a string rather than b, is there anything I could have done more efficiently?

The function `input_number()` should `return CCN;` (Note that the following `printf` statement will never be executed, because the function has already `return`ed.) Then `main` will need to be adjusted to accept the return value from this function, so it can be passed to `luhn_check`. The variable `CCN` is not visible outside the function it is defined in. — Weather Vane, Apr 10 '20 at 17:44
Secondly, you should enter and store card numbers, phone numbers, and PINs as strings. Suppose the user enters `00123`. How will you know if the two leading zeros were entered when the entry was to an integer? Being in a string also makes it easy to access each digit. — Weather Vane, Apr 10 '20 at 17:50
The last line in the question is using the ternary operator. That line is equivalent to an `if` statement: `if (number==0) {count=1;} else {count=log10(number)+1;}` Zero is being handled as a special case. For non-zero numbers, the `log10(number)` will be less than the number of digits because the conversion from floating point to integer drops the fractional part of the floating point number. For example log10(33) is 1.5185... Converting to an `int` results in 1, when the desire result is 2. — user3386109, Apr 10 '20 at 18:06
So, include ```string CCN = 0;``` into main(void) func for it to pass through? Will hop back onto the laptop soon to start fixing things. Does that also require me to convert the string data to a long for the log10 procedure, and back at the end? — schu97, Apr 10 '20 at 18:10
You don't have a string yet: save that for another version. Now, in `main` you need `long CNN = input_number();` but you don't have `count` yet. Assume `count` is one of the correct values for now and pass the correct value to `luhn_check(CCN, 13)` (say) for the satisfaction of getting this working. Then research pointers, and passing them to functions so that a value can be got, and go for the string solution. — Weather Vane, Apr 10 '20 at 18:13
@schu97 If you have a string, you don't need the `log10` procedure to count the digits. The length of the string is the number of digits. Use the `strlen` library function to get the length of the string. — user3386109, Apr 10 '20 at 18:28
Have updated with a complete revision! Thank you! @WeatherVane — schu97, Apr 11 '20 at 08:27
Following the edit: with a string, your can more efficiently a) tell its length, but when there is any leading zero it's impossible with an integer. b) access individual digits, for example the numeric value of second digit (from left) would be `numberstring[1] - '0'`. — Weather Vane, Apr 11 '20 at 08:46

score 0 · Answer 1 · answered Apr 10 '20 at 18:14

Let me start with a few notes on your function input_number:

long input_number()
{
    // this grabs the number and verifies the correct amount of digits
    int count;
    [ .... ]

    while (count < 13 || count == 14 || count > 16);
    // Once this while loop starts, it will never terminate!
    // if count starts at 10 (which is less than 13), the body says "do nothing".
    // so count will continue to be unchanged, and the while-loop will continue to run.

    return (0);  // Code after a return statement will never be reached
                 // So your printf-statement below will NEVER happen.
                 // It also seems unlikely you want to return Zero.
                 // You probably want to return count, or some other value.

    printf("Digits: %i\n", count);       // test print for debugging
}

Made a script on the Luhn's Alg. Unable to understand how to create and call functions clearly for revised script

1 Answers1