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.
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.
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
@@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.
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() 😉
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.
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"
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.
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.
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.
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.
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
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.
@@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).
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.
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 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.
@@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.
@@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.
Рік тому+5
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.
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'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.
@@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.
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.
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.
It would actually be a reasonably complicated addition if you'd like to maintain randomness. I agree with you it would be a nice additional feature, maybe you could add another option for force at least 1 of each character.
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.
@@LowLevelTV 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.
@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.
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
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).
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.
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.
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++;
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 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.
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.
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
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.
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
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 :)
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.
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.
@@LowLevelTV 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.
6:35 Don't be to harsh please. I saw this kind of code earlier. This person has strong ability of problem solving but lack some experinece and low level knowledge. But I'm impressed by way this person solved some problems. Code is not idiomatic or short but is elegant and easy to read.
IMO he was way too soft. The code is bad, it could be written much clearer, shorter and with more functionality (parsing the options instead of using integers). In the video he also didn’t go in depth enough. He explained the concepts enough for someone who is already familiar with maintainability and didn’t bother writing out the new functions. A real learning experience would be refactoring the code continuously, testing along the way, until he was satisfied with the quality.
@@randomizednamme If someone send him code that I suppose want to learn, so being harsh is not necessary. I saw the same flaws but I saw many programs of people who try to solve problem with tool wich they don't know well. So I know this person has ability to solve problems and want to learn, so I try don't intimidate this person. To the second part you're right author indeed could explain possible approach and show how to change it.
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.
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.
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.
Something not mentioned here is that switching on enumerations usually means the compiler will complain if you don't switch on all of them, or if you try to switch on something that is not in the enumeration list. This just helps compile time checking and keeping your code safe
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
@@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.
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)?
I actually had a company tell me in an interview that my code had comments and that was bad. 🙄 I responded that "this is an interview i put them there so you can see my thought process." "No, comments are bad. I can see that you won't be a good candidate. Good bye." This was an online drop shipping company in Albany NY that was connected to a local SUNY college. I guess they just wanted an excjse to not hire someone that didn't graduate at SUNY Albany.
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.
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.
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"
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.
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.
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'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 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.
I don't quite understand why would you want comments in there? I think with bit more declarative naming, and defines for the magic numbers, this is readable code
What strikes me as the biggest red flag is the massive switch / case blob in the first place. If you wanted to add a 5th characterset, or a 6th, it'd add an insane amount of more cases to check for. Also the code assumes the total list of characters can be constructed with just 2 ranges at a time as it stands which only works for 4 charactersets or lower. at 7:44 we can see the string of ptr->chars. Why not define 1 string for each characterset, then construct a new string which is just the sum of whatever charactersets the user selected? Then all the magic numbers disappear, the character sets are easy to define, add to and adjust, and you don't have to define every single case individually. This also makes the repeat functionality trivial. You can just use the constructed characterset without adjusting it to generate the new password.
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.
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
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.
Not a fan of your emphasis on comments, comments are really not maintainable. I think the naming was the actual culprit. And the magic numbers are really very annoying indeed.
Eh mostly agree except when it comes to comments in the code. If functions, parameter names and return variables are named correctly, then there should be no need for comments within the code unless someone is implementing some technical math formula or algorithm. And that comment should pretty much just contain a link. Above the function for API generation is fine too, but in theory it should be redundant because the code itself is readable enough.
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. ... ...
Personally, I am against the recommendation of adding more comments. If your code itself is written in a way that is readable, it becomes self documenting and needs no comments. That means no magic numbers, good function and variable names. Splitting code in functions instead of having 100 lines of coffee in 1 function, etc. The only comments I agree on adding are the function API documents that can be extracted by oxygen. And even those, should only be used for public methods, not internal, private methods.
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.
I completely disagree with having more comments part. The code should be self-documenting. Comments shouldn't be the rule but the exception to document behavior that can't be described by the code itself.
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.
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.
Not a fan of leaving comments that describe how a function is used in the application. Maybe it makes sense at the time, but then something changes and the app works differently. Now maybe the comment is misleading because no one thought to update it. I've been a victim of misleading comments on more than a few occasions, and usually pretty frustrating.
`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.
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 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
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.
wanna learn to code? check out my courses at lowlevel.academy (its on sale!)
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.
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() 😉
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"
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.
charSelectorString
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.
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
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)...
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
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 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.
@@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.
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.
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
@@LowLevelTVagree, 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.
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.
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.
It would actually be a reasonably complicated addition if you'd like to maintain randomness.
I agree with you it would be a nice additional feature, maybe you could add another option for force at least 1 of each character.
there is no reason to use magic numbers if they represent ascii character, just use the characters themselves directly.
@@soniablanche5672 I agree, much more readable, I'm not too familiar with c, can you just replace the numbers with characters and get the same result?
@@sean_haz in C, character literals are integers, they are equal to their ascii code.
@@sean_haz I agree about the reduction in randomness but that decision was made by the websites that set the requirement
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.
@@LowLevelTV 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.
You ain’t a tech wiz if you aren’t using magic numbers.
I see what you did there!
@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.
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
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).
Tip: (almost) never use switch, it can be replaced for smth more readable (almost) everytime
Code review is probably the most helpful thing on youtube for understanding coding.
I am loving the subscriber code reviews. Keep it up.
Glad you like them!
I really enjoyed this particularly how you showed us how to avoid using endless case statements and write smarter. Great job, thank you.
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.
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
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.
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".
This trivial program did not even do what it was supposed to (repeat generation). Thats a fail.
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.
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.
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
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.
At work we never use comments because the code should be so readable that there's no need for it
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
Your channel is so awesome! It's crazy how much I learn in every video!
:D thank you
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 :)
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.
It's "low level" as in "not much abstraction", not as in "low skill level"!
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.
@@LowLevelTV 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.
6:35 Don't be to harsh please. I saw this kind of code earlier. This person has strong ability of problem solving but lack some experinece and low level knowledge. But I'm impressed by way this person solved some problems. Code is not idiomatic or short but is elegant and easy to read.
IMO he was way too soft. The code is bad, it could be written much clearer, shorter and with more functionality (parsing the options instead of using integers). In the video he also didn’t go in depth enough. He explained the concepts enough for someone who is already familiar with maintainability and didn’t bother writing out the new functions. A real learning experience would be refactoring the code continuously, testing along the way, until he was satisfied with the quality.
@@randomizednamme If someone send him code that I suppose want to learn, so being harsh is not necessary. I saw the same flaws but I saw many programs of people who try to solve problem with tool wich they don't know well. So I know this person has ability to solve problems and want to learn, so I try don't intimidate this person.
To the second part you're right author indeed could explain possible approach and show how to change it.
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!
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.
That funky arrayCreation stuff made me feel a certain funny emotion dwelling up inside me.
inline comments should explain why, not what. different from documentation.
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.
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.
You don't have to comment code that is not API
Something not mentioned here is that switching on enumerations usually means the compiler will complain if you don't switch on all of them, or if you try to switch on something that is not in the enumeration list. This just helps compile time checking and keeping your code safe
9:11 ONE PIEEECE, THE ONE PIECE IS REEEEAAALL
"NEVER use raw numbers in your code."
#define FORTY_TW0 42
There, I'm an advanced programmer!
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.
chat is this real?
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
I actually had a company tell me in an interview that my code had comments and that was bad. 🙄
I responded that "this is an interview i put them there so you can see my thought process."
"No, comments are bad. I can see that you won't be a good candidate. Good bye."
This was an online drop shipping company in Albany NY that was connected to a local SUNY college. I guess they just wanted an excjse to not hire someone that didn't graduate at SUNY Albany.
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.
That scoring was generous,
I would probably get fired if I wrote such a code! 😅
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.
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
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"
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.
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.
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.
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 would score it 5, 3, 2, 3, and that is being generous. If I saw it I would just throw it all away and rewrite it as a single straightforward file.
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 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.
I don't quite understand why would you want comments in there? I think with bit more declarative naming, and defines for the magic numbers, this is readable code
Function naming conventions in C are just terrible, what on earth is napms(500)? Nap 500 milliseconds?
Damn, it actually was nap 500 ms 😂
What strikes me as the biggest red flag is the massive switch / case blob in the first place. If you wanted to add a 5th characterset, or a 6th, it'd add an insane amount of more cases to check for. Also the code assumes the total list of characters can be constructed with just 2 ranges at a time as it stands which only works for 4 charactersets or lower.
at 7:44 we can see the string of ptr->chars.
Why not define 1 string for each characterset, then construct a new string which is just the sum of whatever charactersets the user selected?
Then all the magic numbers disappear, the character sets are easy to define, add to and adjust, and you don't have to define every single case individually.
This also makes the repeat functionality trivial. You can just use the constructed characterset without adjusting it to generate the new password.
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.
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.
Suuper helpfull, please more of these!!
You got it!
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’
What window manager do you use?
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
Awesome as always!
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.
Where do I learn this low level stuff (out of curiosity) I am an Android Developer btw
Not a fan of your emphasis on comments, comments are really not maintainable. I think the naming was the actual culprit. And the magic numbers are really very annoying indeed.
Writing good docstirngs makes your code more valuable training data
Eh mostly agree except when it comes to comments in the code. If functions, parameter names and return variables are named correctly, then there should be no need for comments within the code unless someone is implementing some technical math formula or algorithm. And that comment should pretty much just contain a link. Above the function for API generation is fine too, but in theory it should be redundant because the code itself is readable enough.
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!
Can you make a video explaining how NAN and INFINITY works at low level?
Personally, I am against the recommendation of adding more comments. If your code itself is written in a way that is readable, it becomes self documenting and needs no comments. That means no magic numbers, good function and variable names. Splitting code in functions instead of having 100 lines of coffee in 1 function, etc.
The only comments I agree on adding are the function API documents that can be extracted by oxygen. And even those, should only be used for public methods, not internal, private methods.
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.
I completely disagree with having more comments part. The code should be self-documenting. Comments shouldn't be the rule but the exception to document behavior that can't be described by the code itself.
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.
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.
Not a fan of leaving comments that describe how a function is used in the application. Maybe it makes sense at the time, but then something changes and the app works differently. Now maybe the comment is misleading because no one thought to update it. I've been a victim of misleading comments on more than a few occasions, and usually pretty frustrating.
`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.
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.
Good video 💯
Thanks 💯
"Never comment your code. If it's hard to write it should be hard to understand."😏
What's wrong is to use THAT number in your thumbnail
silence from the 8:30 to 8:42 explains how understandable it is :D
MY MAN I wasn't quiet for THAT long. you scared me, I thought premiere pro cut audio out of my video again XD
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
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.