|
I found this code somewhere:
...
if (x == true)
return true ;
else
return false ;
Actual code was the body of a C# 3 lambda expression. Anyway, horrible.
|
|
|
|
|
Could be for debugging, for a breakpoint.
|
|
|
|
|
It's not THAT horrible. Yes, it's not exactly elegant, but sometimes this is done to make things clearer to other coders (especially less-experienced ones).
|
|
|
|
|
AEternal wrote: sometimes this is done to make things clearer to other coders (especially less-experienced ones).
But then how would they learn?
|
|
|
|
|
The senior programmer could place comments like this:
if (x == true)
return true;
else
return false;
codito ergo sum
|
|
|
|
|
Fair enough.
|
|
|
|
|
Then it should be
return x ;
|
|
|
|
|
PIEBALDconsult wrote: I'll have you tied to an anthill and covered in honey!
Cute. This should get a Vote of '5'.
Vasudevan Deepak Kumar
Personal Homepage Tech Gossips
A pessimist sees only the dark side of the clouds, and mopes; a philosopher sees both sides, and shrugs; an optimist doesn't see the clouds at all - he's walking on them. --Leonard Louis Levinson
|
|
|
|
|
or he could just save the time it takes to write that long, verbose comment and just write
return x;
|
|
|
|
|
No time or effort should be spared in the process of education of the younger generation.
Even better would be.
return (x == true) ? true : false;
codito ergo sum
|
|
|
|
|
With longer conditions and expressions, ternary operator becomes tedious to debug.
Vasudevan Deepak Kumar
Personal Homepage Tech Gossips
A pessimist sees only the dark side of the clouds, and mopes; a philosopher sees both sides, and shrugs; an optimist doesn't see the clouds at all - he's walking on them. --Leonard Louis Levinson
|
|
|
|
|
It IS that horrible when you join a project and the other "experienced" consultant is writing a pile of redundant crap like that:
if(x == true)
{
control.Enabled = true;
}
else
{
control.Enabled = false;
}
The above adds an extra comparison operation in addition to being ugly. Besides, as someone already mentioned, How will the juniors ever progress?
Forgiveable for the OP though
The project i joined has a metric buttload of that garbage. Even worse is these guys "architected" classes without methods (can you say pre-OOP structures boys & girls?) and then created a function library (which they called a "fascade" and is NOTHING like the fascade design pattern) where you pass in an instance of this methodless "class" to the method of a fascade class to do work on it (i.e. persist it). The reasoning? They thought adding methods to the class would make it too slow. It has been a rollercoaster of emoticons.
e.g.
class Person
{
public string FirstName;
public string LastName;
}
class PersonFascade
{
public void InsertPerson(Person p) { /* do stuff */ }
public void UpdatePerson(Person p) { /* do stuff */ }
public void DeletePerson(Person p) { /* do stuff */ }
}
|
|
|
|
|
codemunch wrote: class Person
{
public string FirstName;
public string LastName;
}
class PersonFascade
{
public void InsertPerson(Person p) { /* do stuff */ }
public void UpdatePerson(Person p) { /* do stuff */ }
public void DeletePerson(Person p) { /* do stuff */ }
}
Isn't your example standard practice?
Person = Business Object
PersonFacade = Database Wrapper
You get exactly the same when you generate any DB Mapping using an ORM system, and i see nothing wrong with it.
I always operate on the aim to separate data structure from functionality.
That said, the naming conventions are misleading.
-------------------------------
Carrier Bags - 21st Century Tumbleweed.
|
|
|
|
|
Well, the example wasn't too detailed but as-is it took OOP and bastardized it into a function library (PersonFacade) + simple data structures (Person).
You do want your persistance layer (data) to be separate but working at the business layer (often from a different class) and calling data layer specific persistance methods is ugly not to mention a pain in the ass to manage.
A much cleaner solution is to have a Save() method (and depending on your scheme, a Delete()method) which then passes itself to a data layer so in your business & UI logic you don't need to call data layer specific methods residing in a completely different class. The business object takes care of itself and your code is better organized. The data layer could look like the "facade" thing or could be a smarter all-encompasing gate keeper.
|
|
|
|
|
Great point. Never thought of it that way. But it wasn't a sennior programmer making things clearer, nor a junnior one making misstakes. It's just some guy who has been programming for a while and I guess has no clue how boolean expressions work. It's started on C# by the way. Original code
x => if (SomeBoolMeth(x) == true) return true; return false ;
If you were a professor and find this in some test, not from a basic course, what would you do??
Again: Great point!!
|
|
|
|
|
Either you've understood that "(x < 5)" is a boolean expression every bit as much as the boolean constants "true" and "false", or you haven't. There's nothing in between. The code makes it look as if the condition is somehow not a boolean expression, and therefore highlights the author's lack of understanding, of something quite fundamental, and therefore deserves to be on display as a coding horror, in my opinion.
For some reason I see this done a lot with ternaries in C#:
return (s == "toto"? true : false);
Unfortunately proper use is less commonly seen, such as
return (row != null? (int)row["count"] : -1);
|
|
|
|
|
What is x declared as? It's not a var is it?
|
|
|
|
|
Good catch!
Here could be trying (although not recommended) for "object == true".
|
|
|
|
|
what if x was a nullable boolean ?
<br />
static bool getX(Nullable<bool> x)<br />
{<br />
if(x==true)<br />
{<br />
return true;<br />
}<br />
else <br />
{<br />
return false;<br />
}<br />
}<br />
</bool>
|
|
|
|
|
that didn't quite come out right ( first post :P ) was supposed to be a bool after the Nullable of course
static bool getX(Nullable<bool> x)</bool>
|
|
|
|
|
uggh, ok last attempt I am sure you follow ..
Member 4364689 wrote: static bool getX(Nullable<bool> x)
|
|
|
|
|
It still makes no difference. An if condition is a boolean expression and can only evaluate to true or false, regardless of whether the equals operator is working with operands that are structures (such as Nullable<bool>) or objects (such as the object == true mentioned above) or anything else.
It is of course true that the expression "x" cannot in general be known to be equivalent to "x == true", but you can ALWAYS rewrite
if (x == true)
return true;
else
return false;
to just
return (x == true);
And of course, if x is in fact a bool, it would be clearer to simply return x.
|
|
|
|
|
So much for just doing a return x==true;
If x is a bool, then just a simple return of x will work
"I guess it's what separates the professionals from the drag and drop, girly wirly, namby pamby, wishy washy, can't code for crap types." - Pete O'Hanlon
|
|
|
|
|
I considered creating a new thread for this, but (obviously) decided not to.
I spent yesterday at the "Heroes happen {here}" event (in Phoenix) and most of the demoes in the "developers' track" was presented by a "developer evangelist", and it seemed like every one of his methods that returned a bool was written in that form.
|
|
|
|
|
I wrote my first game when I was about 14 or 15, on my lovely Amiga 1200. I did not really know what I was doing, but the outcome was quite good (it was a MadTV clone, but with a radio station). I still have my old Amiga, and just for the fun of it, I took a look at my old code (which was written in CanDo... guess nobody knows it). I was pretty amazed of what I did without knowing what I did, but I stumbled upon a... "concept" that I used pretty often. It basically looked like this (pseudo code):
if not rect.PointInRect(point) or control.IsDisabled() then<br />
DoNothing()<br />
else<br />
... # Some real code here<br />
endif<br />
DoNothing is not a dummy here, it was an actual function. Yes, I had a function that just did... nothing And I used it pretty often. Oh, and the "else" part was not always there. Sometimes I only had the if/DoNothing part (no idea what sense that made to me when I was a kid).
So what tells me this? I was able to get a full game up and running, but knew sh*t about boolean algebra and how these strange if-thingies work. It still makes me giggle.
|
|
|
|