|
That would work better for sure 
BTW, the string literal is just that, a string literal, when there are several 'consts'* defined some lines above
* without any const specifier or special naming of course!
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
|
|
|
|
|
#2 is even worse. The literal on the right is 7 characters long, the string on the left only 3.
|
|
|
|
|
Well spotted, I did stop at the case mismatch there!
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
|
|
|
|
|
I discovered this little gem courtesy of our lead developer.
Application.DoEvents();
Thread.Sleep(1000);
Application.DoEvents();
This is in a loop on the UI thread, called from a forms timer.
|
|
|
|
|
|
Application.DoEvents and Thread.Sleep are a classic combo that I have seen far too many times. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Coded by lazy programmers, for lazy programmers... This is what happens when you sleep at work! 
It's an OO world.
|
|
|
|
|
Just read this story in the lounge[^]. Reminds me of a true shame...
Our 'senior', SQL expert recently wrote some dynamic SQL SP recently.
He is fully aware of SQL injection and actually went about it keeping security in the back of his head.
This is what he came up with (just his idea, not the query)...
sql_statement = 'select '''@param''' from someTable'
I took a look at the query and asked him what in the hell all those apostrophes where doing there (the thing was full of it, lots of magic going on there!).
He said that was to prevent SQL injection. Can you imagine my reaction?
I broke it with a single search for "D'artagnan" 
It was then and there that I (a SQL rookie) thaught him about quotename[^]... 
It's an OO world.
modified on Tuesday, August 30, 2011 10:23 AM
|
|
|
|
|
Now can you get someone to teach you both about bound parameters 
|
|
|
|
|
Very true indeed! I was not aware of them (or perhaps I am, but not of the name). However, I'm not the SQL expert
It's an OO world.
|
|
|
|
|
Parameterized queries are safe in this context!
Well there are those 'Xperts' and 'Seniors! everywhere  Try this[^]
|
|
|
|
|
I don't know - it isn't in my code anymore...
I wrote this wonder this morning, and realized I could prune it down to just the one line:
private void SetRole(SMUser user, UserRole userRole, string roleName, List<string> inRoles, List<string> outRoles)
{
(((user.Roles & userRole) != 0) ? inRoles : outRoles).Add(roleName);
}
I mean, yes it works. But...
It's not there anymore: I changed the whole way I handle roles for other reasons. But still, It's a bit impenetrable...
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.
Manfred R. Bihy: "Looks as if OP is learning resistant."
|
|
|
|
|
In my opinion, it's a nice piece of code, you should take it to the 'Clever code' forum. Readability is a problem though.
|
|
|
|
|
It's the readability I don't like at all - that is not clever, it's just downright nasty!
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.
Manfred R. Bihy: "Looks as if OP is learning resistant."
|
|
|
|
|
I wouldn't qualify it as horror. It's just a little bit too clever.
With a good comment to explain it quickly, or a more explicit method name, it would be fine by me.
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
|
|
|
|
|
I have to agree its not really horrible; as long as it is well commented it is ok.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
If you feel the need to comment the code,then the code is a horror.
Good code shouldn't be dependent on comments to tell the system story.
Or at least that is my 2 cents
That said, this isn't a big horror, it clearly states what it does, 3 nitpick is the use of prefix in the SMUser class name, and that the name of the method is somewhat misleading, and that the method does two things that a) should be clearly specified in the method naming or b) refactored into two separate methods.
modified on Tuesday, August 30, 2011 6:21 AM
|
|
|
|
|
annathor wrote: If you feel the need to comment the code,then the code is a horror.
Surely you don't mean that commenting code makes any code a horror..?
|
|
|
|
|
No, not always, there are some rare cases where cluttering the code with comments are justified.
But if you look at some code and thinks "gosh I better comment it, so other people understands it" then it is an indication that the code is a horror, why do you need to explain the system whit a language that is not designed to describe a system, when you allready have described it with a language that is designed to excplain a system?
Wouldn't it be better to left click, refactor->rename and clean up the code so it tells the story you want it to tell, insted of trying to excplain the story by including a secondary language into your source files?
also comments tends to become misleading and/or misplaced as the system evolves, so instead of excplaining the system it just adds to the confusion.
so, hyperboled, I say, a well commented code is a indication that the system is dirty.
and commenting in itself can also make clean code dirty by breaking up the logic and lie to you while you read the code.
In most cases you could explain what the sysem is supposed to do better whit a programming language than whit english or swahili. And if you, god forbid, uses comments to add todos, changelogs or similar there are plenty of good tools out there for that.
|
|
|
|
|
Comments are beyond important.
As a simple proof, using your IDE of choice, check the intelisense for a command, where does that come from? Comments.
During development, it's a good idea to free-form what a class/variable/method is for, but how? Comments.
Write down what you intended to happen, where you intend it to be, how? Comments?
In short, comments should describe, in the natural language of the developers what the code is doing in the machine language. Simples.
Panic, Chaos, Destruction. My work here is done.
Drink. Get drunk. Fall over - P O'H
OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre
I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer
Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
|
|
|
|
|
I say you're just mostly wrong about that.
Often it's as important, if not more so, to know WHY something is coded like it is as to see the code itself. Comments should tie the parts together and provide a concise lead-in to the code itself. Not to say that logical procedure and variable naming isn't a huge help, but good comments can make big difference.
Of course the unfortunate thing is usually that bad code is accompanied by bad or no comments.
|
|
|
|
|
I agree with you in principle: the code should be readable, so comments, or at least those which simply describe what is being done, should be unnecessary. However, we need comments for two reasons:
- To cover why we are doing it. For simple cases this might be obvious from a function name, but even for trivial calculations, describing why you're doing it is useful. For example, which of these is clearer?
surfaceVolume = volume * 298 * p / (T * 101325);
or
surfaceVolume = volume * 298 * p / (T * 101325);
(Yeah I know I should have consts for those values and in similar real code I do.)
- Sometimes the language feature won't let us express our thought without a bunch of syntax. In a procedural scalar language like what most of us use, this is particularly true for array operations. E.g.
double[] r = new double[a.Length];
for(int i = 0; i < r.Length; i++) r[i] = a + b;
Experienced people might be able to read a loop and immediately process it to what it represents, but many won't be able to. Linq helps with certain categories of syntax (complex foreach loops to filter an enumeration), but array-based maths still needs comments for what you really mean.
|
|
|
|
|
surfaceVolume = volume * 298 * p / (T * 101325);
Or even better...put that formula in a well named function and call it, so your call looks something like
surfaceVolume = CalculateVolumeByIdealGasLaw(volume, pressure, temperature);
Now you don't need the comment. And your code is easier to test.
|
|
|
|
|
Up to a point. Code which contains nothing but several levels of 'well named functions' wrapped around a simple arithmetic calculation are a pain to debug (trying to actually find the places where calculations are done and therefore being able to see whether they're done right). It's kind of like trying to read a book by just looking at the contents page. It's hard to read such long names, too (unless you put in underscores to make it look like a sentence but then the arguments seem to be bound to the last word).
This would go in a function for me if I were using it in more than one place (which in this example I almost certainly would be).
Your proposed name misses the most important point, by the way. It should be called "STPVolume" or "GetVolumeAtSTP" or something like that.
|
|
|
|
|
You think debugging is more difficult with more functions? I disagree. My IDE steps into functions, or over them...which makes debugging simpler, not harder. I can step over functions I've already eliminated as the problem, and into those which could be an issue. Rather than stepping on every line of code.
And I should know if the bug is in a function or not by writing tests for it. My "STPVolume" or "GetVolumeAtSTP" function should have test methods ensuring it's accurate.
As for the name, physics isn't my domain, so I didn't know the proper name, I made my best guess. The essence of my point still stands.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
