What are the worst programming habits?
-
I was thinking about the things that bug me and came up with a short list
- No comments. I know - let's have a religious war etc, but I find no comments dangerous.
- using o as a variable name. In fact using anything that's not sensible.
ctx,dr_rfp_ptr,i2 - Bad formatting. It's like walking into a house and being unable to sit down because of empty pizza boxes on the couch
- Mystery side-effects in code.
- Magic numbers
I'm guilty of 2 of these on occasion. What's your list?
cheers Chris Maunder
-
You should have mounted Andy's head on a pike outside the castle walls as a warning to others.
Software Zen:
delete this;Oh, I was tempted. But I wasn't at all sure whether I could replace him, and I was short-staffed already.
(This message is programming you in ways you cannot detect. Be afraid.)
-
That isn't self-commenting code, it's badly commented code. Those comments are worthless - but that's not to say that some comments wouldn't be helpful
PooperPig - Coming Soon
Agreed. I felt that the comment for UpdateUser(); Should have indicated what information was being updated in which direction... // Grab DB Information and load into object or // update DB with current user information Because I have NO IDEA which direction that update is operating, although I guess I could look :-) But those comments in his example are USELESS comments, which are worse than no comments, because they give you false security.
-
// check if user is valid
if(IsUserValid(user))
{
// update the user
UpdateUser(user);
}
else
{
// show a messagebox with an error
MessageBox(error);
}Worthless comments, agreed...but doesn't mean comments aren't expected. I'd much rather have a comment at the beginning of the code segment saying what this chunk of work means, rather than what each step does...for example:
// better validate user before we get too far into the process...
</twocentsworth>
I used to call it "Super Happy No-Pants Wonder Day"! It turns out that the police just call it "Tuesday". Go figure...
There is a saying that goes something like "you can tell how smart someone is by how much they agree with you". Kudos to every one of these bad coding habit comments! I can tell every one of you has not only developed, but MAINTAINED real life code!!! I would like to make one more statement in favor of good comments. In the classical book by Fred Brooks (The Mythical Man Month), he stated "always throw the first version away". When I am about to code something non-trivial, I almost always write the comments first, as I consider that my "first version". I figure that if I cannot describe in words what I need to do, then I probably can't describe it in code either.
-
Can you explain the reason? :) There is no good argumentation. "This" is used for the nut-cases who don't want to prefix with an underscore, and it is one of the most abused keywords, littering code without adding ANY value whatsoever.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^]
Eddy Vluggen wrote:
prefix with an underscore
X| Yuck, filth, "littering code without adding ANY value whatsoever". X|
You'll never get very far if all you do is follow instructions.
-
I like to be explicit in my code. There is no doubt in my intent. I just find it a bit strange that "private" is the only exception to access modifiers where you have to be explicit about everything but only because it's default. I'm just saying that the access modifier shouldn't have a default forcing you to be explicit in your intent and (granted hopefully) think about what you're doing. I am happy to say, as the other replier pointed out, that I'm not one of those that needs to be "saved from himself" because VB made everything Public by default and that's what generates tons and tons of bad "public everything" code. I don't believe that the problem is with VB. I believe the problem is with the education and the lax standards of what should be taught in school. I've has more than few degreed grads that couldn't tell me the difference between public and private. I've also heard most of those same grads say they've never written an API, to which I call BULLSHIT since every application contains it's own API, usually for the sole consumer being the application itself. I think the entire "private as default" or whatever modifier is default is a Band-Aid on a bigger problem. EDIT: And just for the record, I'm going to admit to being a hypocrite. I also rely on private being the default in my own code but, just because I do it, that in no way means I think it's a good idea.
A guide to posting questions on CodeProject
How to debug small programs
Dave KreskowiakDave Kreskowiak wrote:
like to be explicit in my code. There is no doubt in my intent
Hear! Hear!
Dave Kreskowiak wrote:
"private as default" or whatever modifier is default is a Band-Aid
Testify, brother!
You'll never get very far if all you do is follow instructions.
-
Being overly complex to prove how smart and bleeding edge you are, with the bad variable names and no comments. At least when I did a variant of Duff's device, I laid the case statement over an if/else, I commented what was going on and why. I was young and 'smarter' than I am now. Comments should give the why something is being done or changed. git. I work on source files not directory structures. If I want to check in a single file but have messed around in a bunch of others that I'm not ready to check in yet, don't make me do something with them. (How I miss PVCS and file locking.)
MarkTJohnson wrote:
I work on source files not directory structures.
:thumbsup: That's an issue I have with "modern" version control systems I've had to use (TFS and Subversion). But I think it stems from Visual Studio and other tools that also insist on working with directory structures rather than individual files. :sigh: The tools we use shouldn't force everyone to use one particular technique.
You'll never get very far if all you do is follow instructions.
-
I was thinking about the things that bug me and came up with a short list
- No comments. I know - let's have a religious war etc, but I find no comments dangerous.
- using o as a variable name. In fact using anything that's not sensible.
ctx,dr_rfp_ptr,i2 - Bad formatting. It's like walking into a house and being unable to sit down because of empty pizza boxes on the couch
- Mystery side-effects in code.
- Magic numbers
I'm guilty of 2 of these on occasion. What's your list?
cheers Chris Maunder
Any single method that requires me to scroll (either way) on a 1920x1280 monitor.
-
// check if user is valid
if(IsUserValid(user))
{
// update the user
UpdateUser(user);
}
else
{
// show a messagebox with an error
MessageBox(error);
}Worthless comments, agreed...but doesn't mean comments aren't expected. I'd much rather have a comment at the beginning of the code segment saying what this chunk of work means, rather than what each step does...for example:
// better validate user before we get too far into the process...
</twocentsworth>
I used to call it "Super Happy No-Pants Wonder Day"! It turns out that the police just call it "Tuesday". Go figure...
Normally happens if you write the comments before you write the code. It is just forgetting to remove the obvious comments after the code has been written.
-
I've encountered a similar, but annoying variant of this. There were a whole bunch of functions in my old employer's code base which did nothing but call an almost identically named function, such as:
someFunc(int a, string b)
{
return some_Func(a, b);
}In some cases, this went on for a step or two further, with some_Func calling some__Func (I HATE double underscores in code), which then again finally called the 'real' function someOtherFunc. I suppose it was the result of a messed up attempt at refactoring. It was hugely annoying when debugging.
Spoon Of Doom wrote:
nothing but call an almost identically named function
I've seen that too. It was in just plain C, in some code that represented a layered architecture -- so it was similar to:
BL_GetDate() { return DAL_GetDate() ; }An OOP version might be:F() { base.F() ; }:sigh:You'll never get very far if all you do is follow instructions.
-
Then please show me the way parametrizing a table name, or field names in a query. Of course, you can keep hacking[^].
Or specifying a value for
TOP. But only as necessary.You'll never get very far if all you do is follow instructions.
-
I hope it wasn't provably unreachable, that would have bad implications for the fabric of reality.
I picture things like
if ( name.Length < 0 ) ...You'll never get very far if all you do is follow instructions.
-
Eddy Vluggen wrote:
Both are private
No; the class is
internal. Trying to make it private yields: Error 1 Elements defined in a namespace cannot be explicitly declared as private, protected, or protected internal F:\Projects\ConsoleApplication1\Program.cs 9 17 ConsoleApplication1You'll never get very far if all you do is follow instructions.
-
Eddy Vluggen wrote:
Both are private
No; the class is
internal. Trying to make it private yields: Error 1 Elements defined in a namespace cannot be explicitly declared as private, protected, or protected internal F:\Projects\ConsoleApplication1\Program.cs 9 17 ConsoleApplication1You'll never get very far if all you do is follow instructions.
-
In order of how I have them listed below: 0) Use of VB. 1) Use of Convert and/or ToString rather than casting and/or Parsing. 2) Over-use of Reflection. Not caching and reusing information retrieved via Reflection. 3) Over-reliance on tools, especially third-party tools. 4) Monolithic classes, lack of modularity, non-single-responsibility. 5) Singletons. X| 6) Convoluted concatenation -- a String.Format will be clearer. 6.1) Concatenated SQL statements, when a parameterized statement is better on so many levels. 7) Not leveraging interfaces. 8) Not allowing polymorphism for no apparent reason. 9) Swallowing Exceptions. 10) Posting snippets of code that use uncommon, custon, or third-party classes and expecting everyone to know what they are.
You'll never get very far if all you do is follow instructions.
I like this list.. more to add: 1. Making all member variables public (usually in the context of unit testing.. but also to increase coupling because the original OO design wasn't decomposed correctly) 2. Making all member methods public 3. lack of use of auto_ptr and other more modern mechanisms for properly handling pointer lifetime management. 4. lack of understanding of exceptions, especially their side effects on locking and memory management. (years of my life has been spent on fixing these types of issues..) 5. misunderstanding by value semantics when passing instances of classes across method interfaces. 6. passing container instances by value (eek!). Note these are ALL things I've directly observed/fixed.
-
Eddy Vluggen wrote:
prefix with an underscore
X| Yuck, filth, "littering code without adding ANY value whatsoever". X|
You'll never get very far if all you do is follow instructions.
PIEBALDconsult wrote:
X| Yuck, filth, "littering code without adding ANY value whatsoever". X|
:) There's also a link somewhere in these posts to an article from Joel Spolsky. There have been various ways of indicating that something is a member, as opposed to a local variable. How do YOU differentiate between the two?
class X
{
private int i;
public int I { get { return i; }
public X(int someI)
{
this.i = someI;// further down the road int i = I;}
}Looks better than prefixing the stuff, but there's hardly any hint that it is a private member or a local variable. It is obvious that I must be something public, either a field or a property. It'd also cause trouble with translating to VB. A simple workaround is using
thisto indicate the member;class X
{
private int i;
public int I { get { return this.i; }
public X(int someI)
{
this.i = someI;// further down the road int i = I;}
}The prefix is merely there to indicate that it is a member that is being referenced. I'd be writing the same as below;;
class X
{
int _i;
public int I { get { return _i; }
public X(int someI)
{
_i = someI;// further down the road int i = I;}
}No confusion, and the minimal amount of symbols to convey all the information I want; everything private is recognizable by the underscore, everything public starts with a capital (similar as the recommendations) and everything local start with a non-capital. The value that it adds is that it makes the naming predictable and consistent, makes it easier to spot errors and read the code. It is something that I found that does not cost time, but saves me time.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^]
-
I like to be explicit in my code. There is no doubt in my intent. I just find it a bit strange that "private" is the only exception to access modifiers where you have to be explicit about everything but only because it's default. I'm just saying that the access modifier shouldn't have a default forcing you to be explicit in your intent and (granted hopefully) think about what you're doing. I am happy to say, as the other replier pointed out, that I'm not one of those that needs to be "saved from himself" because VB made everything Public by default and that's what generates tons and tons of bad "public everything" code. I don't believe that the problem is with VB. I believe the problem is with the education and the lax standards of what should be taught in school. I've has more than few degreed grads that couldn't tell me the difference between public and private. I've also heard most of those same grads say they've never written an API, to which I call BULLSHIT since every application contains it's own API, usually for the sole consumer being the application itself. I think the entire "private as default" or whatever modifier is default is a Band-Aid on a bigger problem. EDIT: And just for the record, I'm going to admit to being a hypocrite. I also rely on private being the default in my own code but, just because I do it, that in no way means I think it's a good idea.
A guide to posting questions on CodeProject
How to debug small programs
Dave KreskowiakDave Kreskowiak wrote:
I think the entire "private as default" or whatever modifier is default is a Band-Aid on a bigger problem.
With the argumentation that one needs to save the developer from himself. I'm sorry, but that only flies if you don't have any using-clause in your files. Be explicit in the class that you use, or accept the default behaviour.
Dave Kreskowiak wrote:
I don't believe that the problem is with VB. I
Not? You just explained why it is :)
Dave Kreskowiak wrote:
I've has more than few degreed grads that couldn't tell me the difference between public and private. I've also heard most of those same grads say they've never written an API, to which I call bullsh*t since every application contains it's own API, usually for the sole consumer being the application itself.
Knowing the difference between the access-modifiers would be kinda basic knowledge. And no, unless it is intended to be consumed by the public, it is not a public API. If there's no SDK to download, then you're not building a framework.
Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^]
-
I was thinking about the things that bug me and came up with a short list
- No comments. I know - let's have a religious war etc, but I find no comments dangerous.
- using o as a variable name. In fact using anything that's not sensible.
ctx,dr_rfp_ptr,i2 - Bad formatting. It's like walking into a house and being unable to sit down because of empty pizza boxes on the couch
- Mystery side-effects in code.
- Magic numbers
I'm guilty of 2 of these on occasion. What's your list?
cheers Chris Maunder
Chris Maunder wrote:
What's your list?
1. Building something because the developer wants it rather than because a customer wants it. 2. Assuming that because something is new that it better. 3. Assuming that because something is new is it without fault and requires no time learn to use well.
-
I was thinking about the things that bug me and came up with a short list
- No comments. I know - let's have a religious war etc, but I find no comments dangerous.
- using o as a variable name. In fact using anything that's not sensible.
ctx,dr_rfp_ptr,i2 - Bad formatting. It's like walking into a house and being unable to sit down because of empty pizza boxes on the couch
- Mystery side-effects in code.
- Magic numbers
I'm guilty of 2 of these on occasion. What's your list?
cheers Chris Maunder
I think i'm guilty of the first one, although I usually only comment when it's not clear what the code does or it may have some unintended side effects.
CEO at: - Rafaga Systems - Para Facturas - Modern Components for the moment...
-
I was thinking about the things that bug me and came up with a short list
- No comments. I know - let's have a religious war etc, but I find no comments dangerous.
- using o as a variable name. In fact using anything that's not sensible.
ctx,dr_rfp_ptr,i2 - Bad formatting. It's like walking into a house and being unable to sit down because of empty pizza boxes on the couch
- Mystery side-effects in code.
- Magic numbers
I'm guilty of 2 of these on occasion. What's your list?
cheers Chris Maunder