|
Forogar wrote: Which one to delete when refactoring?
Both.
GOTOs are a bit like wire coat hangers: they tend to breed in the darkness, such that where there once were few, eventually there are many, and the program's architecture collapses beneath them. (Fran Poretto)
|
|
|
|
|
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() ; }
You'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.
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.
modified 24-Jun-14 14:44pm.
|
|
|
|
|
PIEBALDconsult wrote: 5) Singletons.
I reckon valid use cases for singletons. Facade patterns, for example, especially in C++ code.
PIEBALDconsult wrote: 1) Use of Convert and/or ToString rather than casting and/or Parsing.
I lost you there, can you please clarify?
The console is a black place
|
|
|
|
|
PIEBALDconsult wrote: 6.1) Concatenated SQL statements Revoke the programming license of anyone who does this.
|
|
|
|
|
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.
|
|
|
|
|
6.1 - I had to work on code today of a developer that left us last year. He used concatenated SQL statements...
This is where something like Entity Framework comes in handy - let it handle your sql inserts / updates.
There was also a whole lot of other bad coding habits. I blame the university where he studied though, seems like they didn't teach him good coding standards.
|
|
|
|
|
Jacquers wrote: university [...] didn't teach him good coding standards
Coding standards? At university?
GOTOs are a bit like wire coat hangers: they tend to breed in the darkness, such that where there once were few, eventually there are many, and the program's architecture collapses beneath them. (Fran Poretto)
|
|
|
|
|
0)
3) This can be a project killer, good observation.
4) There is no excuse nowadays, there are plenty of tools available to help with refactoring.
5)
6.1) Unforgivable!
Good observations!
|
|
|
|
|
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.
|
|
|
|
|
Quote: 9) Swallowing Exceptions My number 1 pet hate. Should be in capitals.
//BUT THAT IS SOMETHING ELSE THAT'S ANNOYING IN COMMENTS
Plz 4giv me shtng
|
|
|
|
|
If you need comments to explain what the code does, then the code is too complex. Formatting is a matter of taste, and there's a keyboard shortcut to automatically reformat in the VS-IDE.
My worst programming habits;
- Removing the access modifier "private" from code, as it is redundant. Not a bad habit in my book, but apparently in everyone else's.
- Hitting F5 too regularly. Kills productivity if it takes 15 minutes to build.
- Reading CodeProject while building a solution. I cannot stare at the build-screen, especially since it does not provide adequate feedback on what it is doing. If it appears to be waiting for a long time then chances are that it gets killed using the task-manager.
- Coffee. With two suger, and two cups an hour, that adds to 32 lumps of suger.
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
Eddy Vluggen wrote: Removing the access modifier "private" from code
There should be no default access modifiers; the developer's intent should be clearly specified. I don't want to have to guess, and you don't want me to keep asking you. Specify it, and decrease the hit to your own productivity caused by your juniors.
You'll never get very far if all you do is follow instructions.
|
|
|
|
|
PIEBALDconsult wrote: the developer's intent should be clearly specified. It IS clearly specified if it is omitted. It is not some arcane trick, it is not something that causes side-effects, and it improves readability. It is as usefull as typing "begin" and "end" instead of the default scope-blocks. It might take some getting used to, but it conveys the same amount of information using less symbols.
That's kinda essential, and the reason why we are not programming in COBOL.
PIEBALDconsult wrote: I don't want to have to guess If you have to guess at the default access modifier in C#, you should not be writing in C#.
PIEBALDconsult wrote: and decrease the hit to your own productivity caused by your juniors. Should I prefix each class with a complete namespace? Otherwise they'd be guessing at which class it will take
You explain a junior ONCE that everything that does not have a modifier is private. If they come asking, even once, then make them prefix everything. Using "this" and "that", using namespaces, using "global::". Throw in some hungarian systems, so they won't have to guess the type
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
Eddy Vluggen wrote: Should I prefix each class with a complete namespace? Otherwise they'd be guessing at which class it will take You've done it now
|
|
|
|
|
Adding oil to the fire, a practical example;
using System;
using System.Threading;
using System.Windows.Forms;
using System.Timers;
namespace ConsoleApplication5
{
class Program
{
Timer t = new System.Threading.Timer(null);
Timer t2 = new System.Windows.Forms.Timer();
Timer pfld_SysTimrTimrt3 = new System.Timers.Timer();
static void Main(global::System.String[][] strSrgs)
{
global::System.Console.ReadLine();
}
}
} And yes, the "console application template" has an entry point which is implicitly private.
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
Soooo... the class is private? How does that work?
Even I avoid global:: -- by using an alias if necessary:
using MySqlClient=global::MySql.Data.MySqlClient ;
What the heck is a pfld_ ? A pointer to a fixed long double?
You'll never get very far if all you do is follow instructions.
|
|
|
|
|
pfld_ is a pointer to a fixed long double and it ranks under other variables in testing.
Hence, the underscore notation.
|
|
|
|
|
PIEBALDconsult wrote: the class is private? How does that work? Create a new console-application. Look at the access modifier for "Program" and it's entrypoint. Both are private.
Surprised?
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
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 ConsoleApplication1
You'll never get very far if all you do is follow instructions.
|
|
|
|
|
PIEBALDconsult wrote: No; the class is internal . Yes, have just been reminded (a few posts below) that classes are internal by default, only members are private.
..which does obvious make a bit more sense.
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
Needs comments as it's not clear whatr t, t2 and pfld_SysTimrTimrt3 are
PooperPig - Coming Soon
|
|
|
|
|
Eddy Vluggen wrote: explain a junior ONCE
That's once too many.
You'll never get very far if all you do is follow instructions.
|
|
|
|