|
One of the biggest dangers in software development is when code gets into the code base without first going through the human brain.
Un-commenting a block that looks like it shouldn't be commented out is a classic way that this occurs.
|
|
|
|
|
Duncan Edwards Jones wrote: Un-commenting a block that looks like it shouldn't be commented out is a classic way that this occurs.
Never heard of someone doing that. If you thought that was what I was suggesting, you misread.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
No - I'm suggesting it can happen if code is commented out and then the result checked into source control....
|
|
|
|
|
Any code that has implemented some sort of source control shouldn't have any commented code.
If your customer wanted to go back to previous functionality, you should review the source code history to retrieve it from there, it should even be wrapped up in a single check-in with all the dependencies that the functionality relies on.
You can comment code during development to test other avenues, in fact, I think can use whatever coding practice you like while developing  but it shouldn't get committed into the code base.
Sorry if this comes across a bit sour, but I'm working at a place that used to use commented code as source control... They have source control now, but they haven't grasped the concept very well and the code is littered with obsolete, misleading and blatantly wrong comments  
So... crime.
|
|
|
|
|
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
|
it's often handy to leave commented-out code in place if it shows what was already tried but didn't work.
if you're using a third party library, for example, and the docs lead you to think that doing X,Y,Z should work, but after talking with the authors you learn that your situation is special so you really need to do W,X,Z. seeing the incorrect (and well-labeled as incorrect!) code can steer future developers away from making the same mistake.
|
|
|
|
|
RyanDev wrote:
Actually, there have been times when business requirements went back to what they were before and so uncommenting the code was quite simple.
I don't leave commented code in forever though. After a certain amount of time passes, it can go.
I generally like to submit code in as clean a state as possible. My rule of thumb is that if I have committed commented out code at least once then subsequently I will remove it because I know I can get back to it. There are other cases where I might leave it in longer, e.g., if it seems likely that I may need it after some other piece of functionality is available. In that case, I leave a TODO comment explaining why. Should I subsequently learn that it's no longer needed I purge it.
Kevin
|
|
|
|
|
Kevin McFarlane wrote: hen subsequently I will remove it
Which means you cannot search for that code anymore.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
It will be in the source code history.
Kevin
|
|
|
|
|
Can you search through source code history? In Visual Studio, I can choose: this document, current project, and entire solution. I haven't seen anything that allows me to search through source control history.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
RyanDev wrote: Can you search through source code history?
No, you can't but I don't find this too much of a hindrance to look back at my own commits of a file. The way I see it is if I comment out code and I've not left a TODO then it's quite likely that I really won't need this again so I remove it. Possibly it might stay for a couple of commits. But certainly as a consumer of other developers' code I find it distracting when it contains too many blocks of commented out code as you have a tendency to look at what's been commented out and it can break the flow of the code.
If there is some really big stuff that is in progress that I really shouldn't commit yet then I would shelve (TFS) or stash (Git) it.
Kevin
|
|
|
|
|
But where? And why was it removed?
|
|
|
|
|
harold aptroot wrote: Committing commented-out code
We don't do this in our shop - a big no-no. That is what Source control is for.
|
|
|
|
|
Depends on how you do it.
I sometimes comment code with an
#ifdef old_code
#endif
when completely replacing a function's implementation. This way, in the (near) future, if there are some issues with the function, I can see what I (or someone else) did in the original implementation.
(note: in the long term, I do remove that code, but in short-to-medium, I leave it there)
Best,
John
|
|
|
|
|
Well, I always leave the commented out code there.
If I see I any commented out code more than a week old, I delete it.
(because obviously the new code didn't break the system, and the old code can always be retrieved anyway)
(the only reason to leave the commented out code there is, if your commits breaks the build, or crashes the app the next day, you can readily reverse the mod - and you will still remember WTF it was all about).
|
|
|
|
|
OriginalGriff wrote: Crimes:
A) Storing passwords in plain text: CommitStrip[^]
B) Leaving your code open to SQL Injection: XKCD[^]
C) Committing code that doesn't compile.
Crimes
Starting Alphabetically 'numbered' lists
|
|
|
|
|
Normally, I wouldn't - but I didn't want to influence people into the "zero- or one- based lists" argument with both being crimes, probably...
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
|
Code that is complex by how its written rather than by it complexity of the problem
Every day, thousands of innocent plants are killed by vegetarians.
Help end the violence EAT BACON
|
|
|
|
|
Reminds me of my tech leads description of my code...
I still think that it's necessary to write some code using reflection to generate a JavaScript file to be added dynamically during application startup to a web optimization bundle to declare AngularJS references to some Web API Controllers so that I can save myself ten minutes of copy-pasting every few weeks when I need a new Controller.
|
|
|
|
|
Well according to the BBC, the recent Talk Talk hack was a simple SQL injection. This from an 'internet' company. Talk Talk is criminal, sounds right to me.
Committing code that doesn't compile can just be a case of not including a file, so I'd say that was a misdemeanor. TFS will kindly do this for you at its will.
Personally I'd say excessive use of design patterns turning the simple into the multifaceted complex is a crime.
Any type that has the word 'helper' in its title- Crime.
var - Crime
Indentation with spaces - Crime
More than 1 type per file - Crime
Inconsistent naming - Crime
Regards,
Rob Philpott.
|
|
|
|
|
Indentation with tabs: Crime.
Sometimes I will read code in Notepad, and there the tab spacing is just too large.
Within you lies the power for good - Use it!
|
|
|
|
|
PJ Arends wrote: Sometimes I will read code in Notepad,
Not just a crime, but also an obscenity. You can't blame tabs for an editor knocked together one afternoon after a liquid lunch in 1985.
Regards,
Rob Philpott.
|
|
|
|
|
Rob Philpott wrote: var - Crime
why would that be? When using Linq, you often use var.
Rob Philpott wrote: Indentation with spaces - Crime
Where I work, we indent using spaces  Should we be put to death? 
Best,
John
|
|
|
|
|
OriginalGriff wrote: A) Storing passwords in plain text: CommitStrip[^]
Which is why my password is 08A168B215C2f1 - that way I can store it in a public variable and nobody knows that it's not encrypted  ... until now... 
“That which can be asserted without evidence, can be dismissed without evidence.”
― Christopher Hitchens
|
|
|
|
|
Having multiple return statements in a single function. 
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
