|
It wouldn't be a Useful comment then would it
A useful comment is by definition useful 
|
|
|
|
|
Well this is an argument for those form vs function people. You know the ones that I'm talking about. The people who believe that function can be completely divorced of form and still maintain function.
|
|
|
|
|
Don't you watch House - everybody lies.
This is true of comments as well (not always updated when code is changed).
...cmk
The idea that I can be presented with a problem, set out to logically solve it with the tools at hand, and wind up with a program that could not be legally used because someone else followed the same logical steps some years ago and filed for a patent on it is horrifying.
- John Carmack
|
|
|
|
|
I chose "Clear and concise code logic and structure" because it's one of the harder things to correct.
The runners-up are:
"Wise use of Whitespace" can easily be fixed in Visual Studio via "Format Document"
and
"Obvious and intuitive names for variables, classes and methods" can probably be easily fixed in Visual Studio with (global) replacements
|
|
|
|
|
I believe all three of these are equally important. I think it would have been a better survey had checkboxes been used instead of radiobuttons.
|
|
|
|
|
DonDriskell wrote: all three of these are equally important
Yes, but two are easier to remedy.
DonDriskell wrote: checkboxes
Yar, maybe "pick three".
|
|
|
|
|
I agree.
I don't think any of the given answers could be looked-to as THE best way to make code readable. Though Clear and Concise, I think, almost covers two other choices (concise meaning limiting length, as short as possible but no shorter, and clear might reference picking obvious and intuitive names, etc.). I'd add good comments as the other needed item (something more than "set x equal to the function return value" -- and YES, I've seen comments like that in production code). 
|
|
|
|
|
We don't all use tools like Visual Studio. I, for one, type all my code by hand. I've seen too many bad pieces of code from visual studio that either aren't xhtml strict compliant, or violate Section 508 and/or WCAG guidelines.
|
|
|
|
|
squeek wrote: aren't xhtml strict compliant, or violate Section 508 and/or WCAG guidelines.
What? I thought we were talking about code. 
|
|
|
|
|
VS may clean up horizontal whitespace, but not vertical. As has been mentioned in another thread, vertical whitespace/grouping can help break the code into more understandable subsections within a method/subroutine/function.
As for renaming. VS has a better option than global search and replace. If you want to change the name of an object/variable at it's definition, it can find all actual uses of that object and change the reference without accidently changing another variable that a global s&r might change (i.e. two identically-named local variables in two different methods of the same form/page/class).
|
|
|
|
|
I've been thinking about this a lot lately while doing code reviews... The most readable code seems to use these three practices:
- Descriptive names for functions, objects, methods, variables... I'm not sure whether or not that falls under "obvious and intuitive"; depending on intuition always makes me rather nervous. IMHO, names for variables, complex objects and high-level methods should describe an intended purpose, while low-level functions and simple types should describe what they do / are:
StringArray entryColumns rather than StringArray stringsLogFileWriter rather than CSVWithTimestampWriter, LogFileWriter::FlushLog() rather than LogFileWriter::WriteUnsavedEntriesToDisk(), SplitCSV() rather than ParseLogFileLineIntoColumns(), StringArray rather than ParsedLogEntryColumns... - Short secondary branches within functions / methods. I don't have a problem with the odd 200-line function here and there, provided those 200 lines amount to a mundate description of a linear process. But when it involves numerous non-trivial branches, it becomes a headache. If there is no
else to an if, then the body should be a separate function. If the <span style="font-weight: bold;"><span style="font-weight: bold;"></span></span>else amounts to "indicate an error and leave the function" or an implicit "get to the end of the function without doing anything", then the test should be reversed and the function should be exited as quickly as possible, allowing me to skip past failure/no-op modes when i want to read the code that actually accomplishes something, and avoid digging into the function's logic when i'm analyzing failure modes. Pendantic "one return per function" people should be taken out back and beaten with a sack of gotos.  And of course, if both branches are non-trivial, they should each be split into separate functions.
- Judicious use of "Why" comments. This has to be the most-preached, least-practiced recommendation i've ever encountered. The only thing worse than a confusing bit of code with no comments explaining why it is necessary is a confusing bit of code littered with comments describing things that are perfectly obvious from reading the code itself, but none explaining why it is necessary in the first place.
|
|
|
|
|
Shog9 wrote: Pendantic "one return per function" people should be taken out back and beaten with a sack of gotos.
But we don't do that with gotos anymore.
|
|
|
|
|
Logically equivalent conditional blocks still count... 
|
|
|
|
|
Psst. They're called exceptions.
Software Zen: delete this;
|
|
|
|
|
Yeah, true that. I can't count the times i've seen a developer who would never stoop to using goto cheerfully breaking out of a nested loop with an exception or a flag or something equally hard to follow.
They do work beautifully for throwing away big chunks of the call stack though. I don't miss longjmp() / signal().
|
|
|
|
|
longjmp() was the creation of Satan. I had a copy of the XLISP interpreter that was chock-full of those damn things, and it made it a major PITA to debug.
Software Zen: delete this;
|
|
|
|
|
Amen! You probably have a clue as to how many times I'm stuck with figuring out someone's hair brained hops around the code, be it for error or because the person wanted to do it. There are many times I hate being forced into FORTRAN.
|
|
|
|
|
You should write an article about this, if you haven't already!
- S
50 cups of coffee and you know it's on!
|
|
|
|
|
I would have put #3 as #1. If #3 is good, I can live with crappy names, functions, and anything else. A good tool can give you new names. A low-level programmer can shorten your functions, but no one can figure out or reverse engineer what "what's his name" was smoking or thinking at the time he wrote the code!
|
|
|
|
|
I didn't mean the numbering to be a priority, just ordinals for reference. I think they're all necessary for readable code - take one away and yeah, you're stuck reverse-engineering it or guessing or pestering the person who wrote it if you can find them.
|
|
|
|
|
Shog9 wrote: If the else amounts to "indicate an error and leave the function" or an implicit "get to the end of the function without doing anything", then the test should be reversed and the function should be exited as quickly as possible,
This is a tough one.
Doing so means that reading the function from top to bottom allows you to avoid a mental stack push when you go into the "if (!bailOut)" but if you are scanning the function quickly then you can sometimes not realise it has an exit point half way. I much prefer to stick to a "bail out first-thing or stick with it till the end" pattern so you never need to worry about a stray exit in the guts of the function.
Shog9 wrote: Judicious use of "Why" comments
Amen brother.
cheers,
Chris Maunder
CodeProject.com : C++ MVP
|
|
|
|
|
Chris Maunder wrote: Doing so means that reading the function from top to bottom allows you to avoid a mental stack push when you go into the "if (!bailOut)" but if you are scanning the function quickly then you can sometimes not realise it has an exit point half way.
Yeah. Don't get me wrong, i know of plenty of ways to write hellish code while still following my own guidelines.
Actually, one of the best ways is to write code that looks simple and straightforward but manages to disguise key details as part of conditional tests or calls to functions that update global state or otherwise depend on being called in a specific (but unspecified!) sequence.
Sad to say, but my worst code usually arises from some attempt at being "clever", either in the structure or the actual logic. 
|
|
|
|
|
I adhere to this in all of my updates, and convince others to follow my lead when I have to work with others, but I consider this the creme that makes a program easy to go back to and overall taste good. Most people who wrote their code originally in F66/F4 and updated them to F77 have large sections of single letter variables with tons of implicitly declared variables. The same people also seldom used any comments even when they moved to the newer machine architectures. There are a few that did add comments, but they also often added whitespace. Of the ones that actually updated their programs, most of them would leave out comments or descriptive variable names, but some few of these would create good white spacing. With that I can pull out a yellow pad and just write notes of how the program works and rewrite the thing to take advantage of constructs that are no longer obsolete and restructure it so that it is easy to modify without much effort. I guess I'm in a slightly different place than most people in the modern civilized world though.
*sigh* The trials and tribulations of being a rocket scientist AND a OOP software developer in a previous life.
|
|
|
|
|
"Pendantic "one return per function" people should be taken out back and beaten with a sack of gotos."
This rule of thumb is really about making a method functionally coherent. ie it does one thing. A lot of functions like WriteOutData() seem functionally coherent but they aren't, they are doing things like parse my input data, reshuffle it, open a file, write out my data. Its not doing one thing.
And thats what methods returning one thing are about. If your method is taking in "out" arguments (to use c# vernacular) and modifying them inside then its really returning more than one value and this probably means your method is doing more than one thing.
There are examples where multiple return values do work, but for me they are usually really simple methods like
bool Double.TryParse(object inputValue, out double result)
it really has 2 return values, 1 the bool if it is a double and 2 the result.
but generally i don't find this is the case.
|
|
|
|
|
Sadly, in the time of design patterns, rich frameworks, reflection and increasingly dynamic typing, clear and well structured functions are becoming a rarity.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
