|
CDP1802 wrote: mark other's code with the unsafe
You might not be surprised to learn that I compile all my code with /unsafe .
You'll never get very far if all you do is follow instructions.
|
|
|
|
|
Ooh, good thread. Not ones I'm guilty of, but ones I call out in code reviews, that I haven't seen covered elsewhere in this thread:
0. Implementing code that's already part of the framework
1. Explicit path creation (rather than using Path.Combine)
2. Lines that are too long
3. Overly complex lines; these are normally ones that combine numerous statements together using ternary and coalescing null operators (it's a sure sign that someone has Resharper installed).
4. Talking about R#, converting clear code into a muddied part ordinary syntax, part LINQ abomination that you cannot remember what it does so you have to spend 20 minutes figuring out the damn thing.
5. Oh, and while we're at it with LINQ, myList.Where(p=> p.Id == aUniqueId).FirstOrDefault(); This one's a twofer - first of all, if it's a unique number, use SingleOrDefault not FirstOrDefault - you're only getting one value back. Secondly, learn how to use the power of LINQ, that Where statement is redundant so the statement can become myList.SingleOrDefault(p => p.Id == aUniqueId);
6. Not checking inputs into methods for validity.
7. When testing code, not asserting that something has happened.
8. Unnecessary try/catch blocks.
9. Blindly consuming exceptions - I'll have no On Error Resume Next behaviour please.
10. Not checking for null. I once saw a production system go boom because while the main code was properly checking nulls, the code that was reporting out that it couldn't do something because of null values didn't check the value out that it was attempting to log; thus blowing up the system.
That's not all, but that should be enough to be getting on with.
|
|
|
|
|
Pete O'Hanlon wrote: Secondly, learn how to use the power of LINQ, that Where statement is redundant so the statement can become myList.SingleOrDefault(p => p.Id == aUniqueId);
However, using Where(predicate).function rather than Function(predicate) is significantly faster.
Incidentally a straight forward While loop is more efficient than either.
So if efficiency is a concern (and we're not just talking close here, there's a big enough difference that it counts!) you need to be careful!
1.
var found = collectionClass.FirstOrDefault(i => i.Field == searchValue);
or
2.
var found = collectionClass.Where(i => i.Field == searchValue).FirstOrDefault();
or
3.
foreach(item in collectionClass)
{
if (item.Field = searchValue)
{
found = item;
break;
}
}
The results for 100,000 collection with 100,000 searches?
1. 'normal' 78.5
1. parallel 32.2
2. 'normal' 51.2
2. parallel 31.9
3. 29.9
PooperPig - Coming Soon
|
|
|
|
|
How about the SingleOrDefault on the same machine?
|
|
|
|
|
I'm not sure what you mean - the timings were all on the same machine.
PooperPig - Coming Soon
|
|
|
|
|
I was just curious on the performance of SingleOrDefault vs FirstOrDefault as per Petes suggestion.
|
|
|
|
|
Oh - I see. I didn't do that - the figures were from a test I did some time ago when someone kept going through our code base changing all the .where(predicate).something() to .something(predicate) because they said it was more efficient - which I didn't think was the case.
PooperPig - Coming Soon
|
|
|
|
|
Willy nilly #including in large code bases. Then you get...
Q) Why does my build take an hour?
A) Because every source now indirectly includes every header
|
|
|
|
|
As part of No. 4 in your strangely numbered list (starting at 1!)
4a) Putting anything but the bare minimum in Setters/Getters - part of the project I am working on lives in permanent side-effect hell because some getters access the database to get values (and don't cache them) and some setters access otehr properties that access properties that access properties - and all of them have side effects
0. Method Names that don't match their function, or do more than their function
e.g.
bool IsValid(Entity myEntity)
{
if (myEntity.Property = null) return false;
myEntity.OtherProperty = SomeValue;
DbService.Save(myEntity);
OtherEntity= DbService.GetOtherEntity(myEntity.Property);
return true;
}
n.) Double-Negatives
if (!notSaved || !isInvalid)
n+1.) Any code that doesn't look like it would if I wrote it on a good day.
PooperPig - Coming Soon
|
|
|
|
|
Team work often brings out the best (and the worst) in people. My peeves are about devs who indulge in:
1. Sending an email to the dev in the next cubicle instead of simply having a chat
2. Refusing to commit code until it is "perfect"
3. Making working code not work in the name of "refactoring"
4. Spending a week perfecting the latest LINQ statement and being unable to debug or optimise the thing
5. Deciding mid-project to change data access
6. Bitching about FXCop
7. Logging? What logging?
8. Hubris
9. Read the spec - real devs don't read specs!
My two fav's are: Inline braces and swallowing exceptions
|
|
|
|
|
Charl wrote: 2. Refusing to commit code until it is "perfect" There's another side to this. Excessive check ins. You don't need to check in every single change every time you make the change - wait until you have a logical break and then check it in. I've seen people check in every 5 minutes, just checking in things like colour or border thickness changes. It wouldn't be so bad if the next checkin wasn't for the next colour.
|
|
|
|
|
Since I use javascript I'm guilty about 2), but here it is:
6) use of lambda functions inside lambda functions (even inside lambda functions).
7) inline conditions, functions and constructing objects:
var i, o;
for(i = 0; (function() { ... })(); ++i) {
o = new (objlist[i])();
}
Although I never allowed 3). If a tool cannot properly autoformat my code, according to my standards, then that tool is thrown away.
|
|
|
|
|
Checking code into source control that doesn't compile.
"Program testing can be used to show the presence of bugs, but never to show their absence."
<< please vote!! >>
|
|
|
|
|
Neglecting the design phase:
- reverse engineering the design requirements because the design was neglected.
"Program testing can be used to show the presence of bugs, but never to show their absence."
<< please vote!! >>
|
|
|
|
|
2. Short variable names. But usually using my own conventions: Array processing: i for rows, j for columns, v for current value; read/written from/to file: d for data, c for counting, etc.
Why? Because I try to keep 80 columns of code.
C# constants are superfluous (MessageBoxDefaultButton.Button1 FTW!), no way to keep the 80 columns, so I use more descriptive variable names there.
|
|
|
|
|
|
1) Leaving Edits in the code (Edits are messages that often pop up in developmental purposes for our in-house testing)
2) Bad tabbing. Don't blame me, really. I use a different tabbing structure due to the program we use doesn't automatically tab things well.
if (Broken)
then fix.this
else !fix.this
end-if
|
|
|
|
|
Wrong or obsolete comments.
|
|
|
|
|
I've seen the best and worst of stuff, but I've grown so used to it I just edit it out.
There are modern tools to autoformat code in any case, I think the only things I would seriously have a problem with on that list are 4 and 5.
Worst coding pet peeve? Code that isn't written defensively, with a mind to robustness, or that is not fully tested.
I too dabbled in pacifism once.
|
|
|
|
|
Allow me to submit something a bit off-axis: a habit of thought.
In more than one place where I've worked, I've encountered persons so confident in their skills that they didn't bother to test "trivial changes." Such "trivial changes" caused major crashes in important products, more often than I (or they) would care to remember. Inasmuch as for many years it's been a large part of my responsibilities to train young software engineers, it's been the very first thing I've pounded on: there is no such thing as a change too small to test.
Some took the advice to heart, but not all -- and when the bills came due, the incredulity of the sinner at issue was often thick enough to slice: "But all I did was...!"
We're fallible, each and every one of us, from the dunces to the geniuses, and from the brand-new graduates to the fifty-year veterans. But an engineer's ego can be resistant to that homily...until he's experienced the consequences on his own hide.
My "favorite" case of excessive confidence involves a young turk -- let's call him Andy, as that was his name -- who was assigned a component in a large monolithic application intended to run on a VAX under VMS. Andy was excessively fond of assembly language, and was eager to write his piece in VAX assembler. I counseled him against it -- the rest of the application was written in C -- but couldn't dissuade him. To shorten the story a bit, some weeks later Andy presented me with his component, which I added to the build without comment. The resulting application ran for approximately twenty seconds before it crashed -- and it didn't just bring down the app; it crashed VMS with a "bug check" error.
The problem was, of course, in Andy's module. I pointed it out to him at once. The subsequent exchange ran roughly as follows:
FWP: Did you test it?
Andy: Well...
FWP: This instruction [I pointed it out] is out of sequence. You have to allocate and enable mapping registers before it will be valid.
Andy: Well...
FWP: I expected you to test this before you brought it to the link.
Andy: But it assembled without errors, so I figured it was right!
Words fail me, friends.
(This message is programming you in ways you cannot detect. Be afraid.)
|
|
|
|
|
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.)
|
|
|
|
|
I'm all for comments when necessary. I know that is a wide range, but my pet peeve here touches on that range. I think comments can be useful, like if you need to remark on a pitfall about why the code is done that way. Or if you need specific thoughts to consider if you refactor. Stuff like that.
However, when I encounter people who think code should be commented I run into two things that hit my peeve list:
1) comment everything, even if the comment is stupid
2) Don't worry about doing stupid things, because you can comment them. Use the comment instead of good architecture.
Elephant elephant elephant, sunshine sunshine sunshine
|
|
|
|
|
I was working with a horrible C framework wich contains most of the worst programming habits. Very large functions (for about 4000 lines), no coments or coments like /*##@@&& and do something*/ (I swear its real) in a 1000 lines function and things like that. It was like the "1000 dont's about programming"
C, C++, Java, Verilog, VHDL, PHP, and still can´t speak english
|
|
|
|
|
Comments do not match coding logic
TOMZ_KV
|
|
|
|