There are people out there, who say a function is only allowed to have one return statement (called Single Entry, Single Exit Single Entry, Single Exit). In some programing languages like C/C++ it is very useful.
There are people out there, who say a function is only allowed to have one return statement (called Single Entry, Single Exit). In some programing languages like C/C++ it is very useful.
There are people out there, who say a function is only allowed to have one return statement (called Single Entry, Single Exit). In some programing languages like C/C++ it is very useful.
It could be very difficult to give you good hints, because of your restrictions. I would complain that you didn't use functions to encapsulate functionality. Well, but for the future I give you some quotes, why I would use a function in some places.
First of all, you try to determine the number of digits of the credit card number. The way you solved this problem is strait forward, but I think it would be more readable with a log10-aproach. You can read something about log10 or just play a little around. It returns a floating point number which is close to the number of digits. You only need to call ceil for it:
digits = ceil(log10(credit_card));
Here I would use a function called getNumberOfDigits(long long int number), so you don't have to think about your for-loop or my ceil-log10-thing.
Now your switch statement. I prefer if-then-else statements, but I think this is ok here, even though it is very heavy because of all the code in there. I would use functions here to get the body of the switch statement smaller. But we can get is smaller anyway (in a second).
I think it is a bad thing to let the switch statement drop through all cases if, lets say, the credit card number has 13 digits but doesn't fit the next if statement. This leads to bugs! Like in this case. Try this number: 3401234567890
. It is 13 digits long, not 15 or 16, so it could be a VISA card, but the results says its an AMEX card.
The if statements inside the switch statement are very long and doing heavy stuff many times (pow and division are expensive operators. Again, you don't have to think about this now, but in future you gonna get in trouble if you have to do your work with big data in nearly no time).
If someone reads your code, he has to think about the expressions twice before he knows what you are doing. You try to get the highest digits to compare them. But wouldn't it be nice if you can do something like this:
if (most_significant_digit == 3 && second_significant_digit == 4) { ...
This simply tells him what you trying to do. Because you have included the stdio and string library I hope you are allowed to convert a number to a string:
string credit_card_digits;
sprintf(credit_card_digits, "%lli", credit_card);
I think the string type you use is a lie. I think it is a char* but your shouldn't have to think about pointers now, you will learn about them later any way, so we use the string type (if you try this and have problems, just replace string with char*).
Ok, why should we do this? Now you can access each character directly (and you can determine the size of the number with strlen, so you don't have to do the ceil-log10-thing). A string is indexed from the left to the right, so the first letter is the most_significant_digit
of our new if statement. You get the first letter by calling credit_card_digits[0]
. So our if statement looks like this:
if (credit_card_digits[0] == '3' && credit_card_digits[1] == '4') { ...
Not as nice as our first attempt, but you could introduce new variables.
The result of credit_card_digits[0] is of type char
but you can handle it like an integer, except you have to put the numbers in single quotes and the numbers are only allowed to have one digit.
The second part of your program: Again, I would encapsulate the Luhn's algorithm.
The first thing I noticed was the credit_card /= 10
inside the for loop header. I think this is strange, because the credit_card
is no control variable like i
. So I would do this as the first statement in the for loop body.
The second thing were the last few lines. You are doing the same thing three times: print the name of the bank or invalid and return 0. But you could set the name of the bank to invalid if total % 10 is not 0. So you can print the name and return 0 at the end of the main function.
There are people out there, who say a function is only allowed to have one return statement (called Single Entry, Single Exit). In some programing languages like C/C++ it is very useful.
Last of all I want to say: good job. The way to learn how to program is not a short one. I did it without help, and needed more then ten years to get it right (not perfect, but it was ok). But I didn't had internet back then.
I think it is a good idea to ask other people to review your code. You can learn many thinks and you can protect yourself of bad habits.
To learn even more you could read code written by other developers, but you have to search for nice clean code. Not every open source project is suitable to learn good coding ;-)