Advertisement
If you have a new account but are having problems posting or verifying your account, please email us on hello@boards.ie for help. Thanks :)
Hello all! Please ensure that you are posting a new thread or question in the appropriate forum. The Feedback forum is overwhelmed with questions that are having to be moved elsewhere. If you need help to verify your account contact hello@boards.ie
Commenting your code - what, when, where and how?
Options
-
14-07-2011 12:48pmMod note: Split out from this thread as it was diverging but interesting...
Just to be really annoying, can I give y'all the second part of the FizzBuzz test (the one that really counts at any company that does any sort of real product)?
Here it is : Document your solution.
You'd be horrified at how many grads can't do this, but honestly, every time someone on here asks "what programming language should I study to boost my career", the very first one that always jumps to mind is English. Because if you don't document it, you'll probably come across it in a Coding Horrors thread or WTF post someday...0
Comments
-
Just to be really annoying, can I give y'all the second part of the FizzBuzz test (the one that really counts at any company that does any sort of real product)?
Here it is : Document your solution.
You'd be horrified at how many grads can't do this, but honestly, every time someone on here asks "what programming language should I study to boost my career", the very first one that always jumps to mind is English. Because if you don't document it, you'll probably come across it in a Coding Horrors thread or WTF post someday...
Out of curiosity, how would you document the solution? It's such as simple task, the most I would document is the why not the how. And for FizzBuzz, there is no why except for because you were asked too.
Why would you want to document the modulus operator, when the language reference already does?0 -
Procasinator wrote: »Out of curiosity, how would you document the solution? It's such as simple task
If I was documenting it, I'd start with a function header that explained what the code did as a module, concisely (but concise != cryptic shorthand), listing what (if any) inputs it expected and in what format, and what values or errors (if any) it returned. With lines likeprintme &= IIf(i Mod 3 = 0, "FIZZ", "")
I'd have a short comment after it saying what the overall line is supposed to do ( //add 'FIZZ' to the output string if cnt is a multiple of 3 ).
Yes, I know, it seems like you're documenting the modulus operator, but you're not -- you're documenting what you wanted the code to do when you wrote it. Which may not be what the code actually does (you're not a victim of Dunning-Kruger, are you?), which helps in code review in the "real world".
And yes, this seems too simple a test to document. Which misses the point impeccably - because the test is too simple to code in the first place. This is a very small test of your skillset, it's not a real problem, so the solution doesn't have to feel real. It just has to demonstrate your process, that's all.0 -
...and bang. Right there's the first problem. If I had a euro for every time a developer said "but it's so simple..." and didn't document it, I wouldn't need to develop anymore, even if you withheld the euro every time they were right
If I was documenting it, I'd start with a function header that explained what the code did as a module, concisely (but concise != cryptic shorthand), listing what (if any) inputs it expected and in what format, and what values or errors (if any) it returned. With lines likeprintme &= IIf(i Mod 3 = 0, "FIZZ", "")
I'd have a short comment after it saying what the overall line is supposed to do ( //add 'FIZZ' to the output string if cnt is a multiple of 3 ).
Yes, I know, it seems like you're documenting the modulus operator, but you're not -- you're documenting what you wanted the code to do when you wrote it. Which may not be what the code actually does (you're not a victim of Dunning-Kruger, are you?), which helps in code review in the "real world".
And yes, this seems too simple a test to document. Which misses the point impeccably - because the test is too simple to code in the first place. This is a very small test of your skillset, it's not a real problem, so the solution doesn't have to feel real. It just has to demonstrate your process, that's all.
I disagree. If I saw you comment a basic string concatenation, I would believe you were over-commenting.
It usually a bad sign when so many comments litter the code. It means the code can't speak for itself.
While that IIf line looks cryptic, it's part of the core language, and doesn't need to be documented.
Much in javascript:var switchedOn = true; var message = switchedOn ? "It's on" : "It's off";
Any one who has JavaScript experience should know what this does. If not, they need to look at the syntax and operators of JavaScript. The code is not the place to teach developers. Frivolous comments get in way of code readability - they add too much noise.
I agree with a module description, but in the case of FizzBuzz you can literally copy-and-paste the specification.
If you asked me to document FizzBuzz, I'd give you back the piece of paper with the specification. Realistically, your company would have produce it's templates and best practices on how they like there systems documented.
If you wanted me to tell you how would document in code itself (like the method description, etc) that would depend on the language best practices (Javadoc, etc) and in-house standards.0 -
-
Procasinator wrote: »I disagree. If I saw you comment a basic string concatenation, I would believe you were over-commenting.
But it isn't.
It's commenting on what I thought that line of code was supposed to do, in a small test devised so that it could be done in a short space of time.
Feck's sake, I can't test someone in an interview with real code - (a) it'd need a major NDA, and (b) we work with a large codebase (where large means large. As in millions of lines of code), so you'd have to spend weeks getting up to speed with design documents, coding standards and so on before you'd have a fair run at a "real" test.While that IIf line looks cryptic, it's part of the core language, and doesn't need to be documented.
Put it this way. What happens when you come back in six months to that code and three other developers you've never heard of have worked on it in the meantime. Do you remember not only what code you wrote on such a small thing six months ago, but also what you wanted that code to do?Any one who has JavaScript experience should know what this does.I agree with a module description, but in the case of FizzBuzz you can literally copy-and-paste the specification.If you asked me to document FizzBuzz, I'd give you back the piece of paper with the specification.
Look, we work with labs in eight countries around the world containing developers and testers from a lot more than just those eight countries. English is not everyone's first (or second or third) language and people have trained in everything from assembly to cobol to perl to python to C to fortran to sql to brain****; there are millions of lines of legacy code and the project's been in production now for nearly two decades; and we take on new hires every few weeks or months. This is a big, real product, and we live and die by our documentation of the code. You just can't run a project this large and not document everything fully.Realistically, your company would have produce it's templates and best practices on how they like there systems documented.0 -
Advertisement
-
Procasinator wrote: »I'd side more with this:
http://www.codinghorror.com/blog/2008/07/coding-without-comments.html
/headdesk
That column's saying the same thing I'm saying...I'd have a short comment after it saying what the overall line is supposed to do0 -
/headdesk
That column's saying the same thing I'm saying...
Did you read the article? He explicitly says he would refactor the code so that the comments are not needed. The code speaks for itself.
His example:private double SquareRootApproximation(n) { r = n / 2; while ( abs( r - (n/r) ) > t ) { r = 0.5 * ( r + (n/r) ); } return r; } System.out.println( "r = " + SquareRootApproximation(r) );
0 -
Procasinator wrote: »Did you read the article?He explicitly says he would refactor the code so that the comments are not needed. The code speaks for itself.
He's saying that the code should not require comments explaining what the code is doing on a line-by-line basis, that it should be refactored if any particularly gnarly knot of code exists instead of having such comments.
However, he also says that there should be comments explaining why the code is doing things, which is what I'm saying should be in the comments (a comment explaining why the code is doing X is the same thing as a comment explaining what you intended the code to do).
In other words, the comments aren't to explain what the ternary operator does; they're there to explain what your design is doing at that point. That way, if someone reviews your code and sees your code isn't as efficient as it could be, they'll (a) have the context to decide if you're doing it your way for a specific reason or if you just didn't realise the error, and (b) they'll know what you were trying to do there so they can suggest a better implementation.0 -
Sadly when I was coding, I was told not to write any comments - well at least not in Irish anyway0
-
Yes, back when he first wrote it.
No, that's not what he's saying.
He's saying that the code should not require comments explaining what the code is doing on a line-by-line basis, that it should be refactored if any particularly gnarly knot of code exists instead of having such comments.
However, he also says that there should be comments explaining why the code is doing things, which is what I'm saying should be in the comments (a comment explaining why the code is doing X is the same thing as a comment explaining what you intended the code to do).
I think we read different articles. To quote the end of the article:As Sammy Larbi said in Common Excuses Used To Comment Code, if your feel your code is too complex to understand without comments, your code is probably just bad. Rewrite it until it doesn't need comments any more. If, at the end of that effort, you still feel comments are necessary, then by all means, add comments. Carefully.
He says, if necessary, add comments. Obviously that is a judgement call and depends on a lot of factors.
He does say the why is important. But I said the why is important. We have no doubt about the why. But in the FizzBuzz example, there is no why. The why is because the level of graduate developers is so poor we need to test for Programming 101.
The documentation of FizzBuzz is the specification. The user requirement (why) is interviewer wanted to test basic kills.
You could comment about the LCM of 15 in the body of the code, if you are using it. Other than that, it would be superfluous and actually a bad idea.
FizzBuzz should be an example of them being able to write self-explanatory code.
Because all the developers who work in your organisation that don't speak English (as you mentioned) will rely on this first, comments second. And if you add too many comments, they won't be maintained, they will go stale, and they will turn into devices of miscommunication and distraction.
I have a feeling we don't disagree much on when it's good idea to comment. But FizzBuzz is not a good way to judge a potential candidates ability to document/comment code.0 -
Advertisement
-
Procasinator wrote: »I have a feeling we don't disagree much on when it's good idea to comment. But FizzBuzz is not a good way to judge a potential candidates ability to document/comment code.0
-
I don't see any reason why it would be commented line by line, but as an example, this is how I would comment the PHP code I posted in the previous thread. (With linebreaks for readability).
I tend to just use comments to give a brief overview of what I am doing, individual lines are only typically commented by me if I am doing something that I would interpret as complicated. In larger files, individual blocks of logic will also be commented.
[PHP]<?php
/*
* FizzBuzz test
* Print 1 to 100, replacing numbers that are divisible by 3 with 'Fizz',
* divisable by 5 with 'Buzz' and divisible by both with 'FizzBuzz'.
*
* For each number, we check the modulus, a zero result means that it
* is evenly divisible, so we add either 'Fizz' or 'Buzz' to the output as
* applicable. Should a number pass both tests, both Fizz and Buzz
* Will be added.
* If output is empty, fill it with the number.
*/
for($i=1;$i<=100;$i++){
$o='';
$o.=!($i%3)?'Fizz':'';
$o.=!($i%5)?'Buzz':'';
$o=!$o?$i:$o;
echo $o.'<br />';
}
?>[/PHP]Yes, I did just explain one mangled line of PHP with 9 lines of comments :pac:0 -
I don't see any reason why it would be commented line by lineYes, I did just explain one mangled line of PHP with 9 lines of comments :pac:
).
0 -
Eight actually... and if you'd come back to code after six months, two rounds of code review, three junior developer's "tweaking" of that code and having been on two other projects in the meantime, you'd be very grateful to your earlier self for having the foresight to spend the extra two minutes typing those eight lines took (you can touchtype, right?
).
Haa haa, don't worry, I always comment like I did above, and I have quite often been very thankful for my foresight.0 -
The problem is that FizzBuzz (and similar sized tests) have to be that short so you can do them in an interview without study time and NDAs; but that size of test always blurs the line between design and implementation. I think we're both arguing for the same thing here, but it's a fine line between writing a comment explaining a standard language feature and writing a comment explaining your design at that point. In real code, the latter comes out at a 1-2 line comment every 20-30 lines, depending on complexity and boilerplate and so on; but the former is always going to be almost 1:1 between code and comment lines. The trick is trying to see what side of that line someone comes down on in an interview, and it's not an easy one.
But if you ask a graduate to comment the code, surely all they will do is comment every line 1:1 as you say. What does this tell you? I can't see the value.
If you want to make sure they understand the code, and just didn't write it by rote, ask them to explain it.
If you want to know their level of English, you'd be better asking them to write a short essay. It would be a much better display on how expressive they are in written language.
You could also just ask them what syntax options are available to comment in the chosen language. If they don't know this, they probably haven't given a thought to ever commenting code.0 -
Procasinator wrote: »But if you ask a graduate to comment the code, surely all they will do is comment every line 1:1 as you say. What does this tell you? I can't see the value.
If they're writing comments in a way that would be useful in real code, that's something you want to know because it's a major boon.If you want to make sure they understand the code, and just didn't write it by rote, ask them to explain it.You could also just ask them what syntax options are available to comment in the chosen language. If they don't know this, they probably haven't given a thought to ever commenting code.0 -
Eight actually... and if you'd come back to code after six months, two rounds of code review, three junior developer's "tweaking" of that code and having been on two other projects in the meantime, you'd be very grateful to your earlier self for having the foresight to spend the extra two minutes typing those eight lines took (you can touchtype, right?
).
Would he though. I mean, let's take the last paragraph of his comment:For each number, we check the modulus, a zero result means that it
is evenly divisible, so we add either 'Fizz' or 'Buzz' to the output as
applicable. Should a number pass both tests, both Fizz and Buzz
Will be added.
You've completely described the implementation. You even said we have to check to see if modulus is zero.
So 6 months later, after a couple of junior developers of come along and changed the implementation, you're comments probably don't make any sense any more. You've doubled the maintenance on that code.
Why tell that modulus should be zero. Explicity state it in the code:if ($i%3 == 0) { $o .= 'Fizz'; }
Is much more readable. I know you used this code a silly example (like my JavaScript in that thread), but it is much more valuable in code to see readable code than a overly explicit description of the method.0 -
-
Procasinator wrote: »Would he though. I mean, let's take the last paragraph of his comment:
You've completely described the implementation. You even said we have to check to see if modulus is zero.
So 6 months later, after a couple of junior developers of come along and changed the implementation, you're comments probably don't make any sense any more. You've doubled the maintenance on that code.
Why tell that modulus should be zero. Explicity state it in the code:if ($i%3 == 0) { $o .= 'Fizz'; }
Is much more readable. I know you used this code a silly example (like my JavaScript in that thread), but it is much more valuable in code to see readable code than a overly explicit description of the method.
Ohh I know that code fails as an exercise in good coding, purely because of how it's written. That was intentional, it was written to be as short as I could make it off the top of my head, and as difficult to read.
Using the example of junior developers changing the code without changing the comments, I'm not too sure of the validity of that. That can happen in any case, no matter how readable or well commented your code is. If someone changes what it actually does, or how it does it without also updating the comments, then they are bound to be misleading.
Well written code with meaningful function and variable names that is not commented can be much more maintainable than horribly written well commented code. (In my experience)0 -
Ohh I know that code fails as an exercise in good coding, purely because of how it's written. That was intentional, it was written to be as short as I could make it off the top of my head, and as difficult to read.
I know, I did the same in the thread.
But maybe a better exercise would to give graduates a messy bit of code. And ask them to clean it up and explain what the code does.
You'll quickly see if they can write readable code and explain it.0 -
Advertisement
-
Procasinator wrote: »I know, I did the same in the thread.
But maybe a better exercise would to give graduates a messy bit of code. And ask them to clean it up and explain what the code does.
You'll quickly see if they can write readable code and explain it.
The last time I did a technical interview was a couple of years back for a certain company in Dublin.
Mostly it consisted of things like "This code is supposed to do this, but it does not. Why?" and plenty of those multiple choice questions like$i=3; $j=4; $k = $i + $j; if($k=66) { echo 'True'; } else { echo 'False'; }
Does this return A) True,False, C) an error message.
I'm not a fan of those type of questions, I'm not too sure I see the benefit (Over questions like the FizzBuzz etc).0 -
The last time I did a technical interview was a couple of years back for a certain company in Dublin.
Mostly it consisted of things like "This code is supposed to do this, but it does not. Why?" and plenty of those really multiple choice questions like$i=3; $j=4; $k = $i + $j; if($k=66) { echo 'True'; } else { echo 'False'; }
Does this return A) True,False, C) an error message.
I'm not a fan of those type of questions, I'm not too sure I see the benefit (Over questions like the FizzBuzz etc).
I agree, you need to show skills in code comprehension AND code composition. But maybe refactoring messy code can help show this.0 -
Procasinator wrote: »I agree, you need to show skills in code comprehension AND code composition. But maybe refactoring messy code can help show this.
The idea of having them comment their own code is that (a) they've just written it, the structure is in their head and you want to see can they get it down so you can recreate that structure in yours enough to work with it; and (b) they don't get borked by some random code idiom they've never seen before - if you want a language lawyer, sure, that's a decent test, but if you want an engineer, it's not a very useful one. Hell, there's a reason I keep K&R and other reference books on my desk/filesystem, it's because I don't trust my memory to keep every little nuance of the language - that's why we invented writing(and I'm not saying that you don't have to remember what control flow statements there are, I'm more saying that not remembering if >= has a higher, lower or equal precedence to == in ANSI C is not a good test of whether or not you're a good engineer).
And yes, Fizzbuzz is too short; but it's short to fit into an interview's timeslot. Giving much larger, more complex coding tasks isn't as great an idea as it sounds, even if it does mean that the final comment line:code line ratio is closer to what it tends towards in practice. You have to remember that an interview is a completely artifical environment (seriously, do you wear a three-piece suit to your job every day or jeans and a t-shirt? And did you interview in those jeans?) and make allowances for that.
So yeah, you feel silly commenting fizzbuzz. Well, so what? You'd fell a damn sight sillier being paid to code it in the first place, wouldn't you?0 -
I think I am going to try patent the main() function.0
-
And yes, Fizzbuzz is too short; but it's short to fit into an interview's timeslot. Giving much larger, more complex coding tasks isn't as great an idea as it sounds, even if it does mean that the final comment line:code line ratio is closer to what it tends towards in practice. You have to remember that an interview is a completely artifical environment (seriously, do you wear a three-piece suit to your job every day or jeans and a t-shirt? And did you interview in those jeans?) and make allowances for that.
Yeah, but these kind of things become practices. It's fine for me and you to know this, but management might not. Graduates might not. Eventually, we are educating people to comment everything. Then people like Jeff Atwood have to go off and tell people not too! :P
I know it's usually quite difficult to give a good pop-quiz in an interview that will tell you much. But while I might use FizzBuzz, I wouldn't ask them to document it. Simply because I personally see no value from doing so.
If I want to test their English, I can do so. Code reviews/initial mentoring will pick up if they are not following in-house commenting and documenting standards. Even if there is no official documents on the matter, most graduates will follow the example hopefully others are leaving in the code-base.0 -
Procasinator wrote: »Yeah, but these kind of things become practices. It's fine for me and you to know this, but management might not. Graduates might not. Eventually, we are educating people to comment everything.Simply because I personally see no value from doing so.If I want to test their English, I can do so.Code reviews/initial mentoring will pick up if they are not following in-house commenting and documenting standards. Even if there is no official documents on the matter, most graduates will follow the example hopefully others are leaving in the code-base.
Seriously, if you're hiring, and you have the world's leading documentation setup, you don't hire someone who'll damage that while you train them; and if you have a bad documentation setup, you don't hire someone who'll impede efforts to improve it.0 -
Erm. Look, I hate to sound patronising but I don't know a non-patronising way to point out that a lot of us have had to work in places where that would require more hope than even Obama could call for
Seriously, if you're hiring, and you have the world's leading documentation setup, you don't hire someone who'll damage that while you train them; and if you have a bad documentation setup, you don't hire someone who'll impede efforts to improve it.
Well, you don't hire a graduate with no experience and let them commit code based off their FizzBuzz code and documentation. Whatever about regular code reviews or even a mentor, this is the basics. Otherwise, you have big problems.
If you want someone you can just land straight in it you go for someone with experience. And if that's the case, you hopefully wouldn't feel the need to give them a FizzBuzz exam at all.0 -
Here is a test I would give.A programme developer by your team outputted 6 fields in a 3x3 table.
The code is included in a following places, and looks like this:Code: print field1 +"\t"+ field2 +"\t"+ field3 +"\n"+ field4 +"\t"+ field5 +"\t"+ field6; Output: Field1 Field2 Field3 Field4 Field5 Field6
As the system has grown, more fields have been added. The team find it a nuance to update and the process is prone to error.
Every so often, different delimters other than tab and newline are need.
Provide a function that will allow:
An variable-length array of string fields to be printed.
Accept 2 delimeters: a column one and row one.
Allow the developer to specify how many columns for each row.
The function should then print the fields.
Please provide any appropriate comments with your solution.
The other developers will need description of the method and it's parameters so they know how to use it.
Please ask the interviewer if you need clarification or more detail from the specifications.
I'd give marks on code readability and how well they describe the method with appropriate comments. Maybe we disagree on what is appropriate.
I'd give extra marks if they use appropriate method description for the enviroment (javadoc, etc).
I'd also give extra marks if they handle the case where there is empty cells (like a new row every 3 columns, but only 7 items passed in). Extra marks again if they ask me how to handle it before just making an assumption.
It's pretty much a more believable FizzBuzz, and not really much more complicated.0 -
It's not just recent grads who can't comment their code. I know people with 15 years experience who can't and who rely on the "it's self evident" defence.
But if I'm asking during a code review what the hell their code is doing and why the hell it is doing it that way, then it probably isn't self evident, despite their wails to the contrary.
There's no point in getting hung up on it being a simple coding test or whatever. How they approach that should give you a clue on how they'd approach something more complex and that really matters. I would say "it's too simple to need documentation" is not likely to be positive.0 -
Advertisement
-
IMHO I think the fizzbuzz problem is too simple to document line by line but obviously does deserve the original spec included as a header comment. With regards commenting in general there obviously has to be some common sense involved with regard having the good sense to know that programmers of all different standards may be reading code written by someone else eventually and that something which is too simple for programmer A to document may well require a little thought from programmer B etc. whereby a comment would make it easier to digest etc. However you can't possibly comment every single line of code and if you take the stance on this one that the fizzbuzz task should be commented line by line then that implies every sinlge line of code that anyone ever writes should be commented and that's just not feasible. Fundamentally there is no right or wrong answer to this. The only viable stance that can be taken on it is each to their own from a workplace perspective.0
Advertisement