0

So i need help to figure out what's wrong with my code. Its capable of testing all the invalid cases but sometimes misses the other cases. I have added detailed comments to explain my thought process.

Here is a link to the question : https://cs50.harvard.edu/x/2023/psets/1/credit/

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

long get_card_number(void);
int get_length(long cardno);
int calculate_sum(long cardno);
void printtype(int sum, int length_cardno, long cardno);


int main(void)
{

    long cardno = get_card_number();    // Get card number from user

    int length_cardno = get_length(cardno); // Calculate length of the card number

    int sum = calculate_sum(cardno);    // To calculate total = sum_digit + alt_digit + last_digit

    printtype(sum, length_cardno, cardno); // To check what type of credit card it is, based on the different parameters

}

long get_card_number(void) // Function to get the card number from the user
{
    long cardno;
    do
    {
        cardno = get_long("Number: ");
    }
    while(cardno < 0);
    return cardno;
}

int get_length(long cardno) // Function to get the length of the card and to get the length of altdigit*2
{
    int i;
    for(i = 0; cardno != 0; i++)
    {
        cardno = cardno/10;
    }
    return i;
}

int calculate_sum(long cardno) // Function to get sum of the digits
{
    int alt_digit = 0;
    int alt_sum = 0;
    int digit = 0;
    int sum = 0;
    int total = 0;
    int last_digit = cardno % 10;

    cardno = cardno/10; // To get rid of the last digit
    bool Alternate_number = true; // To start with 2nd last digit

    while(cardno > 0)
    {
        if(Alternate_number == true) // Alt digits are the 2nd last digit, 4th last digit, 6th last digit and so on (even)
        {
            alt_digit = cardno % 10;
            alt_digit = alt_digit*2;
            if(alt_digit >= 10)          // To add the first and second digits of the 2 digit alt product separately
            {
                int first_alt_digit = alt_digit/100;
                int second_alt_digit = alt_digit/10;
                alt_sum += first_alt_digit + second_alt_digit;
            }
            else
            {
                alt_sum += alt_digit;
            }
            cardno = cardno/10;
        }
        else    // Digits are the 3rd last digit, 5th last digit, 7th last digit and so on (odd)
        {
            digit = cardno % 10;
            sum += digit;
            cardno = cardno/10;
        }
        Alternate_number = !Alternate_number; // To keep alternating

    }
    total = alt_sum + sum + last_digit; // Adding both of them together along with the last digit we removed at the start
    return total;

}

void printtype(int sum, int length_cardno, long cardno) // Function to know the type of card
{
    int zero_or_not = sum % 10; // To only look at the unit's place to see if sum ends with 0
    int first_digit = 0;
    int second_digit = 0;

    if(length_cardno == 13) // If statement to know the first two digits of the credit card number
    {
        first_digit = cardno/10^13;
    }
    else if(length_cardno == 15)
    {
        first_digit = cardno/10^15;
        second_digit = cardno/10^14;
    }
    else if(length_cardno == 16)
    {
        first_digit = cardno/10^16;
        second_digit = cardno/10^15;
    }

    if((length_cardno == 13 || length_cardno == 15 || length_cardno == 16) && (zero_or_not == 0)) // If statement to print the card type based on length, and the first and second digits
    {
        if((length_cardno == 15) && (first_digit == 3) && (second_digit == 4 || second_digit == 7))
        {
            printf("AMEX\n");

        }
        else if(length_cardno == 16 && (first_digit == 5) && (second_digit == 1 || second_digit == 2 || second_digit == 3 || second_digit == 4 || second_digit == 5))
        {
            printf("MASTERCARD\n");
        }
        else if((length_cardno == 13 || length_cardno == 16) && (first_digit == 4))
        {
            printf("VISA\n");
        }
        else
        {
            printf("INVALID\n");
        }
    }
    else
    {
        printf("INVALID\n");
    }

}

I tried finding out the problem but I couldn't, so I'd greatly appreciate if someone could help me out. I'm assuming that it has something to do with how I wrote the calculate sum function. Here are the errors it showed:

:( identifies 378282246310005 as AMEX
    expected "AMEX\n", not "INVALID\n"
:( identifies 371449635398431 as AMEX
    expected "AMEX\n", not "INVALID\n"
:( identifies 5555555555554444 as MASTERCARD
    expected "MASTERCARD\n", not "INVALID\n"
:( identifies 5105105105105100 as MASTERCARD
    expected "MASTERCARD\n", not "INVALID\n"
:( identifies 4111111111111111 as VISA
    expected "VISA\n", not "INVALID\n"
:( identifies 4012888888881881 as VISA
    expected "VISA\n", not "INVALID\n"
:( identifies 4222222222222 as VISA
    expected "VISA\n", not "INVALID\n"

Edits: 1) Changed first_digit to first_alt_digit and second_digit to second_alt_digit.

  1. Also added + sign to alt_digit = first_alt_digit + second_alt_digit.

  2. Removed the usage of the function for checking the length of alt_digit and used alt_digit >= 10 instead.

King Brain
  • 57
  • 6
  • 1
    What are the errors you are seeing from check50? – Retired Ninja Jul 03 '23 at 20:33
  • Did you try [searching](https://stackoverflow.com/search?q=%5Bc%5D+credit+card) for a similar issue? – Oka Jul 03 '23 at 20:34
  • 2
    Don't use an integer to hold a credit card number. There may not be enough precision. Use a string. – Barmar Jul 03 '23 at 20:35
  • 1
    @Barmar: Although I generally agree with you, in this case, according to the instructions, a `long` is supposed to be used. Actually, a `long long` is required on platforms on which `long` is only 32 bits, such as Microsoft Windows. – Andreas Wenzel Jul 03 '23 at 20:36
  • the cs50 library is used here to get access to the bool data type and to use the get_long function which allows us to get long data type from the user. – King Brain Jul 03 '23 at 20:39
  • I'll update the post with the errors. – King Brain Jul 03 '23 at 20:40
  • Please post the results in the question so you can format it readably. – Barmar Jul 03 '23 at 20:41
  • yes sorry for that....i have added it to the question – King Brain Jul 03 '23 at 20:43
  • you don't need to use `get_length()` to tell if the sum is 2 digits, just use `if (alt_digit >= 10)` – Barmar Jul 03 '23 at 20:44
  • You assign `first_alt_digit` and `second_alt_digit`, but never use them. Should you be using them on the next line? Also, shouldn't it be `alt_sum +=`? – Barmar Jul 03 '23 at 20:45
  • You dont need the cs50 library "to get access to the bool data type". You only need to `#include ` to use `bool`. The cs50 library provides functions like `get_long()`, but it isn't portable to a MS VC compiler. The `long` type is not very useful on a PC because of its lack of portabilty. It's only really useful on a system with 16-bit `int`. – Weather Vane Jul 03 '23 at 20:48
  • Yes thanks for pointing that out Barmar. I have edited my question again. – King Brain Jul 03 '23 at 20:53
  • 2
    Aside: please don't make running corrections to the code posted: this isn't an interactive tutorial. Only change the posted code if it wasn't what you were actually using. – Weather Vane Jul 03 '23 at 20:54
  • Sorry for that....i didn't know that....i wont make anymore changes. – King Brain Jul 03 '23 at 20:56
  • If you need to show us an updated version of your code, then please add it to the bottom of the question as an "EDIT", while keeping the original question intact. – Andreas Wenzel Jul 03 '23 at 20:57
  • I verified the sum with one of the errors and it seems to be correct....guess i made a mistake in the card type function. – King Brain Jul 03 '23 at 21:12
  • I figured out the problem...it has something to do with how i calculated first_digit and second_digit in the last function. – King Brain Jul 03 '23 at 21:23
  • In the chapter "Debugging" of week 2 of CS50, you will learn how to use a [debugger](https://stackoverflow.com/q/25385173/12149471). Such a tool allows you to run your program line by line while monitoring the control flow and the values of all variables. That way, it should be easy for you to see in what line your program stops behaving as intended. – Andreas Wenzel Jul 03 '23 at 21:27
  • thanks for the info andreas, i'll do week 2 tomorrow. I'm glad i at least got to fix the issue. – King Brain Jul 03 '23 at 21:39

1 Answers1

0

The problem is with how I calculated the first_digit and second_digit of the cardno in the printtype function.

I used the symbol ^ to raise 10 to the power of 13, 15 and 16 for each length case. But that is not the symbol for exponential. The correct thing to do here is it calculate exponential using the (long)pow(10,13) function, by including the math.h library.

Also, my initial expression for the second_digit did not take the unit's place after the division and instead took a 2 digit number. So i had to write the expression for second_digit as follows:

second_digit = (cardno/(long)pow(10,14)) % 10;
Obsidian
  • 3,719
  • 8
  • 17
  • 30
King Brain
  • 57
  • 6
  • The function `pow` uses floating-point numbers. Using floating-point numbers [can result in rounding errors](https://stackoverflow.com/q/588004/12149471). Even slight rounding errors are generally not acceptable when dealing with credit card numbers. For integers, I recommend that you write `10000000000000` instead of `10^13`. You can also write `(10L*1000*1000*1000*1000)`. The `L` is important, because an `int` is probably not able to represent the result. It can probabably only represent numbers up to `2,147,483,647`. The `L` forces the type to `long int`. – Andreas Wenzel Jul 03 '23 at 21:59
  • Note that in the next version of C (which is called "C23"), you will likely also be able to write `10'000'000'000'000` instead of `10000000000000`. – Andreas Wenzel Jul 03 '23 at 22:06
  • @AndreasWenzel as the `long` range may be as small as 32-bit. `10LL*1000*1000*1000*1000` makes more sense` as well as `long long` instead of `long` in various places. – chux - Reinstate Monica Jul 04 '23 at 01:56
  • King Brain, "The correct thing to do here is it calculate exponential using the (long)pow(10,13) function, by including the math.h library." --> No. Stay with integer math for an integer problem. [@Andreas Wenzel](https://stackoverflow.com/questions/76607940/need-help-to-figure-out-whats-wrong-with-my-code-for-the-credit-card-question-i#comment135069523_76608238) idea is better. – chux - Reinstate Monica Jul 04 '23 at 01:59
  • @chux: In this case, OP's code is already relying on `long` being 64 bits wide as per the instructions of the assignment, because a 32-bit `long` is unable to store a credit card number. Therefore, the `L` suffix is sufficient and `LL` is not required, in this case. However, you are correct that generally, it would be unsafe to assume that `long` is able to represent `10^13`. For example, this assumption would be incorrect on most platforms that use Microsoft Windows. – Andreas Wenzel Jul 04 '23 at 02:03