This code is far from readable.
Optional braces should not be considered optional.
Omitting these braces leaves your code prone to future errors.
Besides this, I personally find it makes your code harder to parse. I'm so used to seeing loop bodies (or
else) contained within braces, that it becomes easy to mentally skip that block and say "Yup, this is the looping stuff that's happening." Here, without the brackets, it just looks like your code has indentation problems, and it is distracting.
Single-letter variable names are not acceptable.
The only place that they're even remotely acceptable is as
for loop iteration variables, and even this is a purely historical exception to the rule (and I'd argue that
j are in violation of this exception as they're not declared within the
for loop's initialization statement).
- Small variable names don't make your code run faster.
- Small variable names don't make your code use less memory.
- Small variable names don't make your code compile faster.
- Small variable names don't make your executable size smaller.
- Small variable names do make your code harder to read.
An amazing example of this last point is your code base where we have six different single letter variable names all in the same scope (plus another two in the
To the compiler, it doesn't matter what you name all of your variables. It's smart enough to keep up with all of them. I'm not that smart. I'm looking at six variables with completely meaningless names that I'm expected to keep track of.
A variable's name should tell the human reader exactly what it is meant to store.
main. It's not called
You did manage to move the
factorial function out. But there's a lot of complexity remaining in
main that can surely be refactored out into smaller, bite-size functions that can be good names to explain exactly what they're doing. And that'd definitely help with your code's readability.