Using Numbers in Your Code is BAD?!? (low level code review)
Вставка
- Опубліковано 10 сер 2023
- One of my viewers submitted some code that they wanted me to review. Code reviews are EXTREMELY important in programming. You should always be giving them, and getting them, to improve your skills as a programmer.
In this video, I grade the viewers submission on the codes functionality, ease of understanding, if the code is maintainable, and if someone could expand on it.
GO GIVE THEM A STAR ON GITHUB RIGHT NOW: github.com/KanzenTaiwa/rcg
EMAIL ME YOUR CODE: lowlevellearning(at)gmail(dot).com
🏫 COURSES 🏫
Learn to code in C at lowlevel.academy
🙌 SUPPORT THE CHANNEL 🙌 Become a Low Level Associate and support the channel at / lowlevellearning
🔥🔥🔥 SOCIALS 🔥🔥🔥
Low Level Merch!: lowlevel.store/
Follow me on Twitter: / lowleveltweets
Follow me on Twitch: / lowlevellearning
Join me on Discord!: / discord - Наука та технологія
I always though that the code comments should go in the .h file because that is the file which sort of gives the functions which are available to be used.
Yes, IF you write headers, for compiled libraries, and you want to describe the functionality for the public functions. Otherwise comments should go where the code is written.
yeah this is good practice
Am I correct is assuming it basically comes down to editor differences? IDE users that show comments as tooltips probably don't care, so the header has the benefit of being known anywhere it's included... But then console editors like vi that don't have tooltips, in the source file puts it where needed.
I'm asking... I only use vi, so I'm assuming this is why someone would prefer the header.
@@meeponinthbit3466 I think you are correct. I recently started using vscode and it definitely has steered me towards putting function descriptions in the header rather than with the function definition. You then get a very nice tooltip describing the function in any source file that includes the header. I usually prefer to have the function description with the definition, but the convenience of the the tooltip is too great to pass up.
I can see quite a few advantages for putting doxygen comments into headers:
- works with closed source
- there can be multiple implementations of the same header
- doxygen runs faster because it doesn't have to scan any source code
- the header is where people who want to use your library go to learn. If they want to read the implementation they don't need the comment.
My biggest issue looking at this code is how things are named. You touched a little on this with function names, but using Array as the name of the struct being passed around is objectively a bad idea. Container is about the same level of abstract (but much less confusing), however using a name like RandStrContainer would have probably been the best option here because of how descriptive it is. Plus, good names reduce the need for comments (doc comments are always good though).
This is the 21st century, and we have editors with decent code completion: Why not RandomStringContainer? And does it really contain a string, or is it really a string? So, RandomString. PromptUserForAllowableCharacters describes itself. And since this is the 21st century, and we have numerous typing tutors, GenRndStr is simply unacceptable. If you're going to be a programmer, you need to be able to type well enough that GenerateRandomString isn't a problem for you. Re documentation, I like when the name documents what, and the comments document why.
I code in JavaScript, And even I know that highly descriptive names/classes are no substitute for comments, Even by yourself! 🙃
Or just call it ApplicationState. It holds the entire application's state, after all.
He probably intended to get back to it later and forgot. Naming stuff sucks no matter what algorithm you're trying to build. I'm creating a game and attempting to stick to a very rigid naming convention at all times, which makes it easy to copy and paste large chunks of it with just one word swapped for another, but it's just so damn hard to find my way around this clusterfuck of code. It's not because it's not well organized (could be better of course, but it's definitely not the worst), but because it's hard to tell variables from one another at quick glance. So many times I had to undo a couple of changes just because I had been modifying the wrong thing.
Instead of having all the allowable character types in one array and then using magic number indexes into the array for the different types, have 4 arrays of the character types. Then build an array of the types the user wants and make the random selections from there. It makes the function with the large case statement in it redundant, the user can select types in any order, all the magic numbers disappear and its way easier to maintain.
i like this solution!
Yep, this strategy makes it trivial to add for example another class of chars, like foreign language characters.
In this context I would also have you enter l, u, n, s instead of 1234. Why make it harder for the user since that could really be handled in code and an error printed if you get any other input.
I had this idea too and coded it in python in like 30 mins in less than 30 lines of code lol. It way more modular too, literally just add a list of characters, a key and add it to the UI lol.
The user should enter any combination of (a, A, 9, $) as examples directly. So "a9" is lower case and numbers. "$9A" is specials, numbers, upper.
I chose to use "9" since in some fonts number zero and letter "O" are confusing, as are number one and lower case letter "L"
I think a huge problem with maintainability here is the double interval input for generating character list. It works with 4 choices, but the moment there are more than 4, 2 intervals won't cut it (for example "135" will require 3 intervals). Then the array intervals are the main point of confusion, so they probably need to be constants. So the solution is to:
- move the constants inside the function, near the array they are refering to
- make a for-loop to paste in characters
For the "1234" parser, you could just take the number as a bunch of characters, and then checking them one by one if they match 1, 2, 3 or 4 and storing results in a bitfield (essentially what you did in the video), and just passing it to the generator, so no switch-cases are needed and instead handled by for-loop mentioned above.
Also creating a switch statement for each combination of the options is just horrible... Imagine you want to add a 5th option. You need to create about 16 new switch statements. And it grows exponentially. AND it was overlooked in the video. It's not expandable at all.
Since user input is interactive, why not just enable/disable for each type? Like checkbox type with space to enable and disable. Then it's just a case of if enabled add to string.
@@MrJake-bb8bsYou're right. But this person have small experience in C so write on a way that solves a problem and should be praised for that.
Maintainability is learned when you must change or expand your own code after few months or few times in a row. I suppose this are learn projects wich will never be seen or changed again.
I rember my toy projects...oh god...
One letter variables, indentations are for weak, everything in main etc.
This lasted to one day when one of my programs found a more than one buyer....
In days I started to praising idents, meaningful names, modules, const instead of magic numbers and small functions 😅
@AK Yes. Probably the creator didn't want to smash the code into pieces (which I definitely couldn't resist to do lol) and significantly lower the writer self-esteem.
its funny that i dont know almost anything about C and i only code for fun but i still enjoy watching your videos. keep up the good work!!
I appreciate that!
Nice review, mostly agree. Only the suggestion to add comments like : returns void falls to me in the category a = 1 // set a to one
Which is totally superfluous and actually distracting. That the function returns void is obvious… of course if return something more complex, you should describe it…
I think he was laying out a template for the idea because he happened to pick a function that returns void.
Also it allows you to focus on just one place to read what the function does rather than having to notice it doesn't say what it returns then looking to the next line.
Yes it's obvious, but it avoids another recognition, response, and context switch. Very helpful for someone with ADHD.
Function comments should be written in a way that they show up in your IDE when referencing the function, personally I would always specify the return type.
Oh an no, you should not duplicate code in the comments, so don't do:
// if the user supplies 0
if (ptr->length >0) break;
The comment doesnt add anything in this case, and might be incorrect if someone changes the code and forgets to update said comment.
If you must add comment here, better would be just:
// validate input parameters
I agree
As not a very skilled programmer I disagree. (ptr->length >0) is not obvious to me but the comment is crystal clear and also helps me learn
@@gazzat5 you could use your LSP to check the type of ptr and go to the place where its declared. In the struct of ptr there needs to be a comment explaining what each variable is used for.
@@gazzat5 Which is why these kinds of comments are seen with junior dev code, but shouldn't exist in production code.
Again a comment should tell your intend, not how is was done. in this case.
// Validate user input.
if (ptr->length >0) break;
...
This tells the reader that the writer wrote the following lines with the intent to validate the user input, the code will then tell how it is done. This also helps keep the code organized, so if the next update adds one more check of user input, this is where to put it, and after 5 updates it becomes clear that we have so many checks that we now want to refactor this to a function, that most likely will be named validateUserInput() 😉
Yeah the srand bit won't work well if it's in a microcontroller or embedded processor - chances are the whole chip will bootstrap in the same way most times and the millisecond count will be close if not the same, THEN you'd use something different. Only introducing a fair bit of asynchronous-ness to the code, RTOSes, eg blocking on unpredictable IO devices, things like that. The fact as a user on a PC you can choose to launch the program when you want, is where tv_nsec as a random seed works fine.
"blocking on unpredictable IO devices", that does make sense, but ages ago we actually had a problem where a certain program would hang unless a user was logged in. Turns out that a (by now very old) version of ghostscript was using the mouse device to get randomness :D So be careful with that kind of thing!
I like these review videos, please make more.
One thing that wasn't mentioned was the way repeat or start over worked, it could be seen briefly when you scrolled through the code. Basically, the same functions were called again from within arrayOutput, which means that the stack would constantly grow, and eventually you would run out of stack memory and crash. Would be better to have a loop in the main function.
Love the video! Lots of people only encounter their first code review on the job, so it's super helpful to see how people read and use code in more of these videos :)
I'd also love to see more videos on C specific project creation (including compiler arguments, directives, makefiles, directory structure, etc.) for us low-level noobs.
I would love to see some regular code review videos.
There is so much to learn :)
C, C++, Rust :)
I'd love some for ASM too
Great video. I agree on almost everything you said, except 1 single very small part, and that is your criticism on the camelCase style used in the code, because this is really just a personal choice and can vary from project to project. I also say this because C and C++ have no clear standards for this, a different situation from many newer languages that actually do have such standards.
And personally, I like PascalCase a lot as I am also very used to it in due to all the C# coding in my daily life, but I am no stanger to this camelCase style as also used Java, JS and many others.
However I have also become a Linux fan and I fell in love with the Rust language, so I really learned to love snake_case as well, even though it asks from someone to type extra underscores everywhere.
Sure, it really wasn't the casing, but instead using more descriptive function names. eitherCase is_fine.
@@LowLevelLearning Thanks for your reply! Alright, I absolutely agree with you on the chosen names and their descriptiveness! I never write function names like that. They did not even contain verbs, but just nouns, which is confusing and make them sound like a variable, not something that actually performs operations.
Personally I think certain numbers are fine like 0, 1, and powers of 2, or (power of 2) minus 1. How you display them is very important. I always do memory addresses in hex and bitmasks in binary
I like to do bitmasks a 1
@@LowLevelLearningagree, binary is hard to count the zeroes/ones to the position. Just like how 1e9 is better than 1,000,000,000 (without these commas)
I'd at least use a line comment on the side to explain the number. It may feel obvious to me while I'm writing it, but 6 months later when I look at it, I won't remember why it is what it is.
I just feel that something like AND %11110000 conveys intent better than AND 240.
@@williamdrum9899 Agreed, the latter is just really unreadable. Similarly, shifting as above doesn't help you (for example) get bits 3-7, without some ugly left and right shifting. Personally I try to maintain the masks in enums, and use those wherever possible.
Function names should be verbs. `arraySize()` is not a good name. `resizeArray()` is better as it explains the action taken and allows you to use nouns such as `arraySize` for variables.
There's a very few cases where number literals are OK in code, like known value checks - `if (0 == array->size)`. Anything else should be a constant to allow you to give context to the number `#define MAX_WORD_LEN 69` and then `if (arraySize > MAX_WORD_LEN)` gives you context on what you mean with a given value; also it helps refactor as stated in the video.
Ah, so I shouldn't
#define zero 0
if (array->size == zero) ...
?
@@timsmith2525 not as to say you "shouldn't" but I'd flag it in a code review, at least without context to justify it. Question to be answered here is "what value does this bring to the code?". Normally you don't need a `zero` constant unless you foresee the value changing or would like to add more meaning to it than the number itself provides. Integer zero will always be `0`. It's different for NULL, since `#define NULL ((void*)0)` tells you the value is not simply `0` but also a pointer and the intent is to indicate "pointing at nothing" (edge cases exists depending on context).
@@hexadecimalpickleSorry, my bad:
#define zero 0
if (array->size == zero)...
IMHO, magic numbers are fine as long as they're immediately understandable. For example: writing (24 * 60 *60) instead of (86400)
If I am forced to deal with APIs that take seconds as input (as I assume your example does), I tend to define constants like this:
seconds = 1
minutes = 60 * seconds
hours = 60 * minutes
And then you could use 24 * hours instead. Reads basically like English.
Of course, ideally, your API wouldn't take seconds like this in the first place. Unfortunate that sometimes there is no choice.
@@Yotanido yeah, that's pretty much the idea. if you're gonna reuse a value more than once, just define a constant. Makes it super easy to refactor later if needed
Yeah I'm usually good with that. Even better I like to do this:
---
screenwidth equ 320
screenheight equ 200
mov cx, screenwidth * screenheight
---
It's all about communicating intent.
@@williamdrum9899Exactly!! 100% agree with this. Code should be readable to anyone unfamiliar with it
If you're writing a library and you put the documentation in the source files as opposed to the header files, includers of your code can't read it unless they look at the source code. If it's in the header files, LSPs can access it too and you can get inline documentation with your language server
Your channel is so awesome! It's crazy how much I learn in every video!
:D thank you
That's amazing, I pretty much comments like crazy because I often come back to my code trying to understand wtf was I doing. 😅
This would've been easier to write and use without ncurses: Enable the character sets, specify length etc with command line options, and print just the random string (following the unix philosophy).
Then the user can do stuff you didn't think of with your program, like append the random string to a file: ./prog >> somefile.txt
You could probably avoid allocating memory for the array entirely by just printing the characters one at a time, as they're generated, in a loop (putc).
One should first of all use good descriptive names, which will make the code easy to read. If there are still things that are not obvious, add comments.
Then it is important to actually use the variables for their explicit purpose. In this example, ptr->size is used to store the seed for the random generator?!? This will definitely cause issues when someone is modifying the code.
Another thing that I note is that everything is put in a single large struct. This is not much better than having everything as globals. The struct both contains members called size and length, without specifying what they refer to, which is confusing. I think it would be much better to have it separated, so that the user inputs have their own struct (making it easy to insert the options from another source such as the command line), the string of candidate characters have their own struct, etc.
I really enjoyed this particularly how you showed us how to avoid using endless case statements and write smarter. Great job, thank you.
I am loving the subscriber code reviews. Keep it up.
Glad you like them!
I always recoil a bit when I hear "more comments", as my experiences lines up well with the idea that a comment is a lie waiting to happen. Renaming those functions/variables in a reasonable way and breaking things up into smaller functions means less to parse, and (while I despise the phrase self-documenting code) it becomes readable without. For maintain and expand, every function having a 5 line comment saying "this function called get array from pointer takes a pointer to an array and returns an array contain the content of the array at the end of the pointer" really grinds down reading (in this case, the comments take up lines that could be used to display more code to give the reader context etc).
Overall I mostly agree with things, magic numbers should live in a global const with a clear name or an enum type (perfect for switch statement use and quick reading), excessively compressed naming and the others things you pointed out are fair.
I agree with the other comment that the header is a great place for the documentation comments. Perfect place to live in the interface file for the code
chat is this real?
Do you have a video of your vim setup? I really like how you were able to jump to function definitions, and tabs on vim
9:21 depends on what you mean by secure. The srand/rand functions implement a pseudo-random number generator, which has a predictable result pattern, a known distribution and the results can be reproduced with the original seed, so if security is a concern, a cryptographically secure random number generator should be used instead, like reading from /dev/random, or using a library that provides an implementation of one with good enough entropy to be considered one...
Agreed. srand takes a 32bit seed I believe, and from then on it is deterministic. So this can at most generate 2^32 different passwords (the equivalent of a ~6 character alphanumeric password). Realistically even less, because not all seeds can occur.
No inside function comments makes things worse. Hard to maintain later.
But i agree headers needs comments as you said.
Comments can be bad too. I’ve returned to code I’ve written with long passage comments, and I have no idea what I was conveying so I just delete the wall of text. More helpful comments to me are the ones where you’re answering why you would do something that way, in a simple one-liner.
@@wandrewp I was not talking about comments inside a function. I was mentioning about comments above the function explaining input, return value, panics, stack overflow errors
A comment on password generators in general. When a password requires a combination of uppercase, lowercase, numbers and symbols it often needs at least one of each so the password generator should reject any proposed result that doesn’t have at least one of each.
Quick Vim tip: you can have both number and relativenumber on at the same time so instead of the 0 you can have your line number.
I dont think that function method commenting is useful unless you want to use some tool to generate documentation. In this case the comment doesnt add anything useful:
This function does X
function: arraySizes
param: ptr, pointer to the array object
return: void
vs
void arraySize(Array *ptr)
and 12 lines of simple code
The only thing I would change is the function name, something like promptArraySize would be better.
I find the more clear and concise my function names are I comment way less
@@johm0151 yep. Comments (in code) should be only used when the code is really complicated and then some refactoring might be better. And perhaps if automagic tools want function definitions to be commented.
Most comments in code are either useless or explaining some complex concept in a short essay. Which means either the comment was unnecessary or ir should be part of the documentation. The code itself should document itself. There are separate files for anything else that is needed.
@@FinkelfunkThat is very true. Most of the time comments are needed for the autodoc generation and nothing else.
Sometimes, just sometimes, the code has to be complex and a comment might explain why it is so, or just mark it as complex thing (Seen useful comments saying "This is the fastest way to do this, don't try to refactor, we already tried several times")
One other use for useful comments is to add some documentation about 3rd party function/rest/etc calls, things that the developer might not have access to.
Ah yes, and regexes should be commented, if you are allowed to use them.
Really depends on the comments, the main purpose of a function comment is do tell the intent, of that function.
If the source is your only reference, then how do I know if the overflow I found on line 23, was a bug or a feature?
I do agree that the example shown is of low value, for one I newer repeat the function name in the comment, that's just redundant.
Code review is probably the most helpful thing on youtube for understanding coding.
An important thing to keep in mind is that comments are often code smells. This happens because in a lot of cases where you would want to add a comment, the next step would then be to make changes to the code such that what your comment was explaining would be so obvious from the code itself that you would not need to have a comment explaining it.
For example, that line comment you make at 4:39 could also be handled by having the section of code be moved to a different function, that has a name that explains that that is what it does. Even for the documentation comment above the same function, all of the information in it could be well contained in good naming of just the header line, the return is there, the input with a good name could also strongly indicate what form it should be, and the name of the function could clearly describe what the function does.
Using it for documentation is a different thing, but as just being a comment it would be a codesmell as written. The main case where such things are needed is when you need to explain something more detailed than what can be given into the name - maybe you want to specify that some input must have a special structure, or some other thing, and in such cases it makes a lot more sense to write out full descriptive comments.
So what kind of comments belong to this category of codesmells? Mainly comments explaining what the code does or what names mean. Comments that serve other purposes, such as remind you why something was done some way or calling tricky parts out is a lot less smelly.
I agree that comments explaining what the code does are bad; comments should explain why the code was implemented this way.
My head nearly explodes when I see comments like this:
// Increment x
x++;
Thanks. If I don't know what "++" means, I really don't have any hope of understanding the rest of your code, do I?
I'm okay with this:
// Point to the next character of input.
x++;
Lol when I started my first job I did things like this. "// This is a function" (not really but also not NOt really)@@timsmith2525
One thing to add to your great post.
Comments can be wrong.
If a developer refactors the logic of a program the dev is bound to refactor functions variable names etc. But nobody forces you to revisit your comments. So over time they get further and further away from the truth, till they are outright wrong.
And at that stage comments become a riddle not a description. "What did the ancient programmers try to tell me".
I know that this is "low level learning", but my version in C++ that uses std::random_device (which is /dev/urandom) and std::uniform_int_distribution to make single-use passwords is 27 lines of code. It was about ten minutes' worth of coding. It's almost always a characteristic of novice programmers to make things complex where they shouldn't be and simple (meaning not extensible) where they shouldn't be, either.
This looks like a fun training project to me as somebody who can't write C but has been working with 6 digit LOC PHP / Python code bases for the last few years. Meaning the structural parts are obvious but I get to fight the compiler and feel like I'm chewing crayons while failing at it :)
On the usage of magic numbers, of course I agree with your view on avoiding numbers as far as possible. But there are three ways in which they could be avoided in my opinion : enums, macros and consts with a meaningful name. Could you share your views on which one would you choose and for what reason? In my understanding, if I had too many numbers to replace, I would be cautious in replacing all of them with enums in order to save stack/data section of the memory. I would use macros too wherever possible as they are literally just place holders that burn zero stack. But thats my view. Kindly share yours.
You didnt show why the repeat functionality works only once :(. Nevertheless great video
ahhh crap good point, sorry about that. i suspected it was because of how he was seeding srand but I cut that bit out.
I’ve had annoying instances where the user input didn’t “take” unless I flushed stdin or it was requiring a line feed or something. Differs based on embedded uart driver
I prefer seeing something like "static const int a = 5;" (or "const int a = 5;" inside an anonymous namespace in C++) instead of "#define A 5". It preserves type information and isn't a magic number during debugging. Modern compilers will optimize away the variable in release mode.
What window manager do you use?
It would be great to make a code review of the source code of DWL. Its a Wayland compositor in 2500 LOC. Love this series ❤❤
Great suggestion!
That funky arrayCreation stuff made me feel a certain funny emotion dwelling up inside me.
@5:47 This is an example of why I believe good programmers need to be lazy: If this had been my first idea, once I realized that I needed to generate all those permutations in my head, I would have devised an easier-to-implement method.
Where do I learn this low level stuff (out of curiosity) I am an Android Developer btw
Can you do a code review where you take code that is mediocre and clean it up? I have done some of those myself and it's pretty fun and a good challenge.
Awesome as always!
I agree with the comments you made about comments (lol). I include at least one-line comments to describe a function’s purpose placed on the line right above it, and the code inside the function will have various comments as well if new memory is needed or local variables are declared and used or a loop or switch-case/if-else conditionals need to be done. I don’t want to over-flood the code with comments, so I also try to take advantage of white space to separate function calls from variable declarations/initializations, and further on that separating variables using a line of white space by their datatypes, with each variable getting a trailing comment giving a condensed description of its purpose so as to provide some extra clarity without taking up more space than the white space already does. It makes the code seem long, but in reality it’s not, if you removed all the comments and white space, probably shrinks to 2/3 of the size it was before doing so. I usually write up all the comments after I’ve written the code, though, a habit I should probably start working to break since it might bite me in the ass if I end up leaving something for a week and didn’t write comments for myself.
10:11 - adding information to seed is not a problem. If a seed is unpredictable, if you add information to it, it’s still unpredictable. Potential issue with doing calculations on the seed is that you’re end up removing information from the seed. If tv_nsec is close to its maximum value, if you multiply it than you’re loosing entropy. (Though I don’t believe this is an issue here in practice). Reason not to add predictable data is that it complicates code unnecessarily.
Also, for general question, libc’s random number generator is definitely not secure. To properly implement this the easiest way (on Linux at least) is to read /dev/urandom. This is especially true since tv_nsec may have much smaller resolution than one nanosecond. This means that there will be only handful values to check.
"Select the option specified below to generate [ascending order]:"
Nope. The grammar was bad, and the method of selecting options is bad. A better way:
"Select one or more options specified below to generate [any order]:
"L - lower [a-z]
"..."
Then lowercase the resultant string and test for specific letters in the string in the code. This needs more code to do the work, but is far less finicky and arbitrary for the user.
Suuper helpfull, please more of these!!
You got it!
Love these. Googled research and leasons dont show what NOT to do, and you juat have to infer any best practices. Its even worse when the example you lookup is over simplified.
Magic numbers are bad practice but by seeing 26 and 52 my guess was the 26 lower and 26 upper letters of alphabet and not their ascii values. Later in video you can see that there's a string containing those characters in alphabetical order. This also explains 62 which would be including 10 digits and probably 30 special character for 92.
Can you make a video where you’re working on a project with a friend? It would be very interesting how professionals work as a group.
I would improve the code by letting the functions return an input if they succeded or failed, if a function returns void then we assume it can't fail when maybe it actually can.
Also I would check if that the malloc didn't fail, make sure the pointer we got isn't null.
I would cast the malloc, makes it more clear what the type of the pointer is.
The program creates a composite array of valid choices then randomly selects from that.
But if memory is limited (as on a microcontroller), there may be merits in generating a random number then TESTING if the number generated is in any of the 4 categories that the user selected (lower, upper, number, special). Possibly with lots of "rejects"
Actually the magic number problem is a really hard one i did see even in the automotive industry they simply don’t name things they also sometimes don’t give you descriptions of the protocol or a certain thing has just the same name as the number in the description. So you have 0x67 at index 2 but there is now info about the why. Which then makes naming without excessive idea reversing not even a option.
Processors are complex. It's amazing to see someone who kicks this stuff out like this. Pure God givin natural talent. Your future is so bright, you're gonna have to wear shades.
I was working as a contractor for a very large international company in a project where the style rules included basically "no comments anywhere". Comments were probably allowed in the file header but nothing elsewhere. The different kinds of style requirements that you run into are kind of interesting. Maybe the most charitable interpretation of this rule was that maybe it forces you to make the code itself clearer.
I worked at a place like that, too. The reason was that the lead programmer was trying to make himself irreplaceable. That didn't work well for him. It did teach us to use descriptive names. We were translating uncommented PASCAL to C. Following a style convention using good names with comments is a much better way of doing things. Outdated comments that don't agree with the code are one of many excuses given for not including comments.
Can you make a video explaining how NAN and INFINITY works at low level?
I prefer function comments in both the header and source file. Sometimes when debugging, if I can read what a function does in the header file it saves me slogging through the source.
I didn't understand the structure type_t at the end, it's the first time I've seen the "
It's shift, shifting left the one bit of one N times.
As someone above me said it's a shift left operator.
Imagine number 1 in binary: 0b0000_0001. Shift left would move every single bit in number 1 left N times. So for example 1 is the same thing but moves every bit to the right.
Where can I submit code so you could consider review?
I prefer to write self-documenting code over adding separate comments. Since I pretty much always use graphical IDEs with code completion, having long variable, function, and class names doesn't bother me.
put the other magic numbers as const in my opinion. as well as the enum for the input
same thoughts here
Hell you could do constexpr since the values won't change anyway
Just remember in C `const != constexpression`
@@martin_hansen am perfectly aware of that, pretty sure results in faster compile times i think
That scoring was generous,
I would probably get fired if I wrote such a code! 😅
Could you do a low level tutorial on doxygen, would be sweet!!
I only use comments when I do something complex or are forced to do something I don't want (because external library limitations). And even than comments are usually white noise to me. I prefer accurate naming over comments
Pretty sure I would be terrible at helping new programmers after watching this video because for the first 8 or so minutes I was like "the person who submitted has to be trolling, right?" and, no, I do not think they were. I have just been a programmer long enough that good ways to do things are embedded in my brain and seeing the (working!) result of someone closer to the "fumbling around in the dark" stage of learning programming hurts my brain.
What terminal theme is that?
somehow youtube sent me back to this vid. I don't use C on daily basis but I would implement this with a hashmap (dict whatever). This way you don't have to change every case for every change. Everything is sorted and in the same place, and changes and usage is dynamic.
One of my work colleagues wrote this: #define FORTYFIVE 45. He said he removed the constants😂
omg
7:55 If I'm not mistaken, `malloc` is unnecessary. We can statically preallocate memory whose size is bounded by the hardcoded max charset-size, which is just `26+26+10+symbol_count` (less than 127 bytes, pretty cheap!). This is faster because 0 syscalls, and safer because all bytes are initialized to 0 at load-time (.bss "magic")
Interesting, but I don't quite get it. Would you please provide a specific example. Also, malloc on modern, multi-user systems will initialize to 0, any way; if you don't trust it, use calloc. Is your idea to, for example, char[100] input_buffer rather than input_buffer = malloc(100)?
@@timsmith2525 correct
9:11 ONE PIEEECE, THE ONE PIECE IS REEEEAAALL
You look young or really learned and achieved mastery in C in 21 century?
What pushed you to this and what you learned from or whom?
I love your adventures in high level languages!
Maybe lovers of them want to scream but for me wich have some taste of both it is funny and eye opening.
7:20 Imo it's a personal preference and maybe stems from the language you came from. Being a C# dev I personally don't really like reading C with snake_case, camelCase and PascalCase are more familiar and pleasant to me, so his naming style seems absolutely fine to me.
Absolutely. The point of a coding style is to pick one you like, and stick to it [where "you" may be "your team", or (IRL) "your manager"] ...And the coder here has done that - that deserves kudos, not criticism.
Sadly I have a bad habit of just doing whatever. I try to stick to constants being in all caps snake case, functions in camelCase and variables in PascalCase but I don't always remember to do so
My eyes/brain deal better with underscores. And, this is the 21st century. Can we please get over shouting constants SUCH_AS_THIS?
#define maximum_string_length 50
is so much better than
#define MAX_STR_LEN 50
@@timsmith2525 You would hate my code then lmao
@@williamdrum9899i also use different casing for different stuff, but mine are PascalCase for functions and types, snake_case for variables and UPPER_SNAKE_CASE for constants. I adopted this style because it is used by a game i like, funnily enough.
Good video 💯
Thanks 💯
You seem to put a great deal of weight on comments. Comments don't execute and in my experience (been doing this since the days of punched cards), often create more problems than they solve. It is extremely rare for comments to be updated when maintenance is done therefore there's a good chance they'll mislead.
I understand the sentiment here. It wasn’t JUST lack of comments, but the comments and lack of understandable function names. I agree, the code should self document with function names and variable names. If it doesn’t, supplement with comments.
@@LowLevelLearning Personally, I would prefer if you had emphasised this angle in the video. I largely agree with your comment here but I would go further and say "The code should self document with function names and variable names. If it doesn’t, -supplement with comments- the issue will be raised during peer review and must be resolved before the PR is merged."
Comments are not unit testable and they cause a maintenance burden. If there's an especially complex piece of logic that seems to need a comment then refactor those lines out into an appropriately named private function. The indirection will be optimised out by the compiler and the improvement in readability is highly valuable.
Tip: (almost) never use switch, it can be replaced for smth more readable (almost) everytime
Immediate mode op codes in assembly enters the house.
I've only watched very few videos of you but seeing that you graded this a 6/10 in terms of understandability has me truly worried about what kind of atrocities you had to look at.
I would've probably rated that a 3/10.
I agree in general with your comment about "magic numbers", and the case situation with options of 1, 12, 123, 1234, etc, etc. But you kept calling out the 26, 52, etc as being magic numbers and asking what would happen if they changed.
I didn't get to see all of the code, but it looked to my like those values were position markers for the string that contains the set of available letters, numbers, and symbols - 1-26 contains the lowercase letters, 27-52 the uppercase letters, 53-62 the numbers, and the remainder - up to position 92(?) the symbols.
While it would seemingly make more sense to have each option contain its own string variable (a-z, A-Z, 0-9, etc) and then concatenate them based on user input, I don't think it makes sense to assume that those numbers might change in any normal course of maintenance (barring a rewrite).
Am I wrong?
The maintenance-problem here would probably arrive if we decided that we (eg.) would ban the 7 characters "1iIlLjJ" - that's a LOT of code to go through and tweak (without making a mistake, or missing a line, or forgetting it also got used once in some other file) ...I would suggest banning confusing/misreadable characters is a "reasonable" change to the code, and does constitute a "rewrite".
@@csbluechip That's fair. But if you are using this code to generate passwords, presumably you are also using something to store those passwords (who is going to memorize 10+ character strings of randomly generated letters, numbers, and/or symbols?).
If it's passwords, you wouldn't *want* to remove the visually similar characters (what about O, 0, Q?) - you might even want to weight them to appear more often.
A password storage system will allow you to copy and paste without consideration for what the characters look like, but a bunch of similar-looking characters in a password will help deter "shoulder surfing".
That's my guess, anyway.
Great video I definitely think some of it is within the eye of the beholder like for example how your name vars nonetheless great video should do this for rust
i would use structs and enums and maybe add some comments, and this code could be fine. Also the names could be worded better.
I'm not necessarily in camp "document everything", unless it's weird, esoteric or super complicated/involved.
I can see what's going on by just reading the code, what's often lost is WHY you're writing it a certain way.
I write code in paragraphs. Each paragraph starts with an empty line and a topic sentence. Then, I can outline the code before I start writing it.
// Prompt the user for the length of the string.
...
// Prompt the user for the allowable characters.
...
...
@@timsmith2525That's not a bad way to do it if someone happens to be skimming!
How do you use more than one c file for a single program
you need to compile every file separately with the -c flag (gcc/clang). This will turn them into object files (.o)
Then you link the object files into an executable with ld
ld -o ./output file1.o file2.o … fileN.o
@@U20E0 Oh I already found out how but thank you for the information
26 isn’t a magic number in this case. It’s obvious that it’s the number of letters in the alphabet. Nobody needs a global variable named NUM_OF_LETTERS_IN_ALPHABET
Lack of context or hidden derivation is what makes a number magic.
From the comments is looks like a video about how to comment would be nice.
My 2 main rules about adding comments.
1. Explain the intent, not the implementation.
- You can read from the code itself how it is done, but not if it is doing as intended.
summedDeg = addDegrees( 200 + 200 )
summedDeg is now 400, seams to work, and if I inspect the code I can confirm this is what was implemented.
Problem is that the actual intention of this function was to add angles, and should overflow at 360,
so correct answer would be 40. With no description of the intent, you won't have a clue about this.
2. Do not repeat values or implementation.
a. If documenting a function, don't repeat the name of that function.
b. Don't create dual source, by repeating a number, often the comment is not updated when the number is
Bad: sleep_ms( 240000 ) // 4 minuttes
updt. --> sleep_ms( 250000 ) // 4 minuttes
Better: sleep_ms( 4 * 60 * 1000 ) // minuttes * secs/min * sec/milli
updt. --> sleep_ms( 3 * 60 * 1000 ) // minuttes * secs/min * sec/milli
Best is to put all that into a const expression of well named defines.
c. Even better give the intent.
sleep_ms( 240000 ) // Wait to let pressure stabelise.
does anyone know what terminal he uses?
i3wm, i3-sensible-terminal which just maps to the ubuntu terminal, solarized dark, zsh with oh-my-zsh and pl9k, neovim with onedark by joshdick
@@LowLevelLearning thank you!
for simple functions I prefer no comments instead you describe the intent of it with just the name of the function and name of the arguments.
IMO using the array like this just leads to unreadable code.
It would be much better to have a function to get the requested size from the user which returns an `int`, then another function which gets the requested charset, then pass those in where needed.
```
uint getDesiredLength();
char* getDesiredCharset();
void printRandomString(int length, char* charset);
```
For example is a much more clear set of functions.
I get the feeling I'd understand this a lot better if I knew what a magic number is, but since I don't this question wasn't really answered for me
In programming, a magic number usually refers to when a programmer sets a specific constant without adequately explaining why it has to be that value. They're chosen for a specific reason that the code relies on to work properly, but without communicating that reason it might as well be magic.
The developer could have improved it somewhat by creating a variable or macro for their magic numbers, though that doesn't address the concern of using magic numbers in the code. A comment explaining how they chose those specific numbers would make the code much more readily understood by a third-party looking to understand what the code is actually doing.
Or, since they seemed to be using the ASCII character codes for the upper and lower bounds, they could take advantage of a C feature that allows single-character literals to be treated as integers. They're typecast implicitly to the appropriate integer type (in fact, C considers `chars` to already be an integer data type, hence the common practice of using `unsigned char` to refer to single-byte non-negative integer values), and putting the actual character value in the code would help readability massively without any comments (for people who hate writing comments), because the definition itself would imply the reasoning that led the developer to choose those values.
I generally use CMake when the program has more than 1 file lol
EDIT: weird how there's only like few function calls in main and that's it
my main.c generally has >100 of lines if i try to make anything bigger lol
EDIT: after seeing the case statement i got a heart attack, it would be so easy to just implement it properly
`arrayCase` just looks like a normal day at work to me... but, hang on, no there aren't any switch statements inside that huge switch statement.... it's nicer than the stuff i have to deal with at work.
This trivial program did not even do what it was supposed to (repeat generation). Thats a fail.
When is your "C Programming Zero to Hero" course coming out?"
Plain C is supposed to be snake case!
That's the way It's have gone too, used bits to represent the options. Also, just to point out at 1
I dont like a lot of the code in general for many issues you mentioned, but if you have to use menus like this, you should parse the input snd then translate it into an enum, or whatever the C alternative is called, its been a long time, like 15y...! But anyway the big switch still iant great but at least your cases are then UPPERCASEALPHABET, SYMBOLS, NUMERIC etc you get the idea. It would at least make the bad way of doing things more maintainable
Ah you did something similar a few mokwnts after i posted this lol
Id say the ascii magic nums are ok btw with a small comment at the top
Expandable 8? If you want to add new groups of symbols you need to go and define summatorially increasing cases in the switch
Why start athe enum at 1
Because in that moment I forgot how to math
I use a lot of numbers in x86 assembly coding.
"Never comment your code. If it's hard to write it should be hard to understand."😏
it's wrong to use numbers in a c code 'like this' of course, quite intrinsically, in every single book from K&R to King, the very first thing you are introduced with is the #define preprocessor meta. So the (integer char) numbers could very well be defined as ASCII_a ASCII_z etc. Use it ppl! :D
You can also just write ‘a’ and ‘z’