Coding - so what's a crime and whats a misdemeanor?
-
Was just adding something in QA and I thought: there are things no sentient coder should do these days, but every day in QA we see some halfwit doing them. So I figure we need a list of Crimes and Misdemeanors, and these are my first candidates. Misdemeanors are "smack on the head" offenses, Crimes deserve a death sentence! :laugh: Misdemeanors: A) Ignoring existing standards and modifying someone else's code "your way". Crimes: A) Storing passwords in plain text: CommitStrip[^] B) Leaving your code open to SQL Injection: XKCD[^] C) Committing code that doesn't compile. Anyone want to add to these?
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
Misdemeanor: Mixing tabs and spaces. Pick one or the other. Not using a Linting tool. Crime: In addition to not checking user input, not validating parameter input for a routine. Not checking for Null. Having two routines do the the same thing.
Jeremy Falcon
-
Eddy Vluggen wrote:
I heard that often
Lets say my application wants to see which localized satellite assemblies are installed, so it gets the names of all of the directories in the installation folder and checks which names correspond to valid Country-Culture. The
CultureInfoclass does not have a method to check if the name is valid. The way to check is to useGetCultureInfoon the directory name and catch the exception if it fails. (It could have returnednullif it fails but it doesn't!)"Fairy tales do not tell children the dragons exist. Children already know that dragons exist. Fairy tales tell children the dragons can be killed." - G.K. Chesterton
-
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 :wtf: :mad: So... crime.
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.
-
Eddy Vluggen wrote:
I heard that often
Lets say my application wants to see which localized satellite assemblies are installed, so it gets the names of all of the directories in the installation folder and checks which names correspond to valid Country-Culture. The
CultureInfoclass does not have a method to check if the name is valid. The way to check is to useGetCultureInfoon the directory name and catch the exception if it fails. (It could have returnednullif it fails but it doesn't!)"Fairy tales do not tell children the dragons exist. Children already know that dragons exist. Fairy tales tell children the dragons can be killed." - G.K. Chesterton
That is not an unexpected exception; you are actively catching the exception and continuing on a known path. That is, if you catch exactly that exception, and not just every exception. Otherwise you might miss it if the user does not have read-rights on that location; that makes for bugs that are difficult to solve, if it is unknown that an unexpected exception is occuring and therewith changing the logic of the application to some unexpected state.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)
-
Eddy Vluggen wrote:
Throwing ex;
As I understand it there's no difference between throw and throw ex in Java, but there is in C#. I suppose that will catch out anyone moving from Java to C#.
Kevin
-
That is not an unexpected exception; you are actively catching the exception and continuing on a known path. That is, if you catch exactly that exception, and not just every exception. Otherwise you might miss it if the user does not have read-rights on that location; that makes for bugs that are difficult to solve, if it is unknown that an unexpected exception is occuring and therewith changing the logic of the application to some unexpected state.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)
Eddy Vluggen wrote:
That is not an unexpected exception; you are actively catching the exception and continuing on a known path. That is, if you catch exactly that exception, and not just every exception.
Yes, it's the swallowing of top-level exceptions that are potentially nasty. I've had to maintain code in which top-level exceptions are swallowed and they swallowed serious errors. To track them down I then had to wade through tons of code. :(
Kevin
-
GOTO can be useful and there are moments where it is needed. Misusing it can result in spagetti code I know but... I would like to see you coding in LAP (PLC) or assembly without JMP instructions...
M.D.V. ;) If something has a solution... Why do we have to worry about?. If it has no solution... For what reason do we have to worry about? Help me to understand what I'm saying, and I'll explain it better to you Rating helpful answers is nice, but saying thanks can be even nicer.
There's an example in Code Complete (first edition at least) where they provide a code example using Goto. Apparently almost no-one was able to rewrite the code correctly without using Goto. :)
Kevin
-
harold aptroot wrote:
Committing commented-out code.
That's called good practice. :-\ 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.
There are only 10 types of people in the world, those who understand binary and those who don't.
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
-
Having multiple return statements in a single function. :thumbsdown:
There are only 10 types of people in the world, those who understand binary and those who don't.
Having a single return is a guideline that I mostly follow. First I aim for short functions so it tends not to be a problem. Secondly returning initially after a condition check and then again at the end is common and fine. Sometimes it actually reads better that way. But long spaghetti methods with returns all over the place is definitely a no-no. Again, I once had to maintain some code in which the single return rule was followed slavishly. The code was mostly pretty impressive but in one case there was a bug caused precisely by slavishly adhering to this rule.
Kevin
-
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.
-
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
-
Having a single return is a guideline that I mostly follow. First I aim for short functions so it tends not to be a problem. Secondly returning initially after a condition check and then again at the end is common and fine. Sometimes it actually reads better that way. But long spaghetti methods with returns all over the place is definitely a no-no. Again, I once had to maintain some code in which the single return rule was followed slavishly. The code was mostly pretty impressive but in one case there was a bug caused precisely by slavishly adhering to this rule.
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
-
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.
-
Me too. I've used that method as loop control where the loop needed to go in sequence (0,1, ...) but it may be missing values. Another was for a HTML form that sends an arbitrary number of arguments - which I sequenced the naming convention but didn't have in an array. In the latter case, the exception was the only way to find the end. But these were always deliberate.
"The difference between genius and stupidity is that genius has its limits." - Albert Einstein
"As far as we know, our computer has never had an undetected error." - Weisert
"If you are searching for perfection in others, then you seek disappointment. If you are seek perfection in yourself, then you will find failure." - Balboos HaGadol Mar 2010
-
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
-
Was just adding something in QA and I thought: there are things no sentient coder should do these days, but every day in QA we see some halfwit doing them. So I figure we need a list of Crimes and Misdemeanors, and these are my first candidates. Misdemeanors are "smack on the head" offenses, Crimes deserve a death sentence! :laugh: Misdemeanors: A) Ignoring existing standards and modifying someone else's code "your way". Crimes: A) Storing passwords in plain text: CommitStrip[^] B) Leaving your code open to SQL Injection: XKCD[^] C) Committing code that doesn't compile. Anyone want to add to these?
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
GOTO yourRoom. ;P
Will Rogers never met me.
-
I'm just waiting for the first person to put "uses JavaScript" or "uses VB".
Whatever happened to that whole dissing VB thing? We didn't all inadvertently grow up, did we? :sigh:
cheers Chris Maunder
-
Was just adding something in QA and I thought: there are things no sentient coder should do these days, but every day in QA we see some halfwit doing them. So I figure we need a list of Crimes and Misdemeanors, and these are my first candidates. Misdemeanors are "smack on the head" offenses, Crimes deserve a death sentence! :laugh: Misdemeanors: A) Ignoring existing standards and modifying someone else's code "your way". Crimes: A) Storing passwords in plain text: CommitStrip[^] B) Leaving your code open to SQL Injection: XKCD[^] C) Committing code that doesn't compile. Anyone want to add to these?
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
Not freeing your pointers.
cheers Chris Maunder
-
Was just adding something in QA and I thought: there are things no sentient coder should do these days, but every day in QA we see some halfwit doing them. So I figure we need a list of Crimes and Misdemeanors, and these are my first candidates. Misdemeanors are "smack on the head" offenses, Crimes deserve a death sentence! :laugh: Misdemeanors: A) Ignoring existing standards and modifying someone else's code "your way". Crimes: A) Storing passwords in plain text: CommitStrip[^] B) Leaving your code open to SQL Injection: XKCD[^] C) Committing code that doesn't compile. Anyone want to add to these?
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
I can't believe you missed the "u" in "Misdemeanours". From what I know, Brits love "their" grammar. :laugh: My additions: Misdemeanours: n) Unnecessary async wrappers for synchronous methods n+1) Abusing
var(for C#) n+2) Under-usage ofsealed(for C#) Crimes: n) Spawning unnecessary threads n+1) Usingdynamicfor "predictable" types (for C#) n+2) Making everything "public" with no thoughts whatsoeverI ain't got no signature.
