Code Readability Poll

[LimitedAtonement]

Dear Mr. Nischalke, When I implement code from somewhere, that's when I wind up editing it. For instance, using the RectangleF structure, I noticed to get 'Bottom' to be a smaller number than 'Top' (exemplar gratia in an Euclidean space), I would have to rewrite the code. When copying and pasting code from the internet, when looking over my old code, that's what I mean by all the time. What do you mean, Not checking an object after doing a cast? I think I checked it with ReferenceEquals . Do you mean that revisiting code that works is a waste of time? Or refactoring Java to C# is a waste? Or casting the object twice (in is and in as) is the waste? In Christ, Aaron Laws

[PIEBALDconsult]

No need for the trinary operator. Otherwise, 5.

[N a v a n e e t h]

Kevin Marois wrote:

Also, one other thing we teach is having only one return in a method

IMO, this makes the code hard to read. Multiple return is the preferred approach. Martin Fowler has a topic on this in his Refactoring book. I don't have it right now to quote. :)

Navaneeth How to use google | Ask smart questions

[PIEBALDconsult]

Mark Nischalke wrote:

casting the object twice

Whether it casts twice or not, I agree there shouldn't be any need to use both is and as on the same object.

[N a v a n e e t h]

I too thought the same after posting. Probably more cleaner approach would be

Insets insets = obj as Insets;
if(insets == null)
return false;
return ((top == insets.top) && (left == insets.left) && (bottom == insets.bottom) && (right == insets.right));

:)

Navaneeth How to use google | Ask smart questions

[Not Active]

N a v a n e e t h wrote:

There is no casting

You're right, poor choice of words on my part. :-O You explained what I was meant though

---

only two letters away from being an asset

[PIEBALDconsult]

N a v a n e e t h wrote:

This is micro optimization

No it isn't; the overall effect on performance may not be huge, but using both is and as on the same object is a sign that the developer doesn't understand what as does, and that's worse than the minimal hit to performance.

[Not Active]

IMO, this is more readable

if (insets == null)

than this

if (ReferenceEquals(insets, null))

LimitedAtonement wrote:

I cast the object first without asking about the cast.

Not checking an object is unforgivable for a professional developer. Refactoring must be balanced with need. Just because you can dosen't mean you should. The code may be working perfectly fine and the refactor introduces defects downstream. I've also had developers waste time refactoring when they had other tasks to perform. If you follow coding standards this sort of readbility refactoring won't be necessary.

---

only two letters away from being an asset

[PIEBALDconsult]

Hear hear!

[PIEBALDconsult]

N a v a n e e t h wrote:

Martin Fowler

But is he the only one who says that? I prefer to make my own decisions rather than follow the word of one author.

[PIEBALDconsult]

foreach (Point a in _gr_pts)
{
if (prev != Point.Empty && prev != a)
{
g.DrawLine(_ap_pen, prev, a);
}

prev = a;

}

[Ennis Ray Lynch Jr]

East Coast Brackets, I like the term. I usually just call it Java-style but yours is more fun.

Need custom software developed? I do custom programming based primarily on MS tools with an emphasis on C# development and consulting. A man said to the universe: "Sir I exist!" "However," replied the universe, "The fact has not created in me A sense of obligation." --Stephen Crane

[DaveyM69]

The only use I've found for ReferenceEquals is null checking in classes that overload operators == and !=.

Dave
Generic BackgroundWorker - My latest article!
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Why are you using VB6? Do you hate yourself? (Christian Graus)

[Luc Pattyn]

:thumbsup: I never associated my bracket placement with Java, and I remember having had similar east/west discussions when being an active Java developer. I checked a few Java books and found both styles in use. I've got the East Coast name long time ago, can't remember where from; and Google seems to have forgotten all about it too. But I continue to use and apply it. :)

Luc Pattyn

---

I only read code that is properly indented, and rendered in a non-proportional font; hint: use PRE tags in forum messages

---

Local announcement (Antwerp region): Lange Wapper? Neen!

---

[Gideon Engelberth]

I've not read enough books (or this one by Fowler) to know if other people say it, but I would personally prefer to use multiple return points. The reality is that in some cases, such as when you have cleanup code that must run and a try-finally isn't appropriate, you may need to have only one return point. Other than that, there is little to no mental benefit to doing it either way. When I read this,

bool bReturn;
if (condition)
{
//do stuff
bReturn = true;
}
else
{
//do stuff
if (other condition)
{
// yet more stuff
bReturn = false;
}
else
{
//blah blah blah
bReturn = true;
}
}
return bReturn;

I still have to trace through the same amount of code to know what the function will return each case, whether I use single or multiple returns. Allowing multiple returns makes it obvious when some condition will stop execution of the method immediately. If I must have a single return, these conditions end up causing if-else statements that I have to trace to the end of the method before realizing that, "Hey, this condition stops the method." This will also usually end up leading to deeper nesting, which is more difficult to read.

[LimitedAtonement]

Dave, But why would a class overload the == operator and not handle null itself? Do you see this often? I also thought this, but reconsidered recently. The reason I was using ReferenceEquals is because I don't want to have to check to see if it has an overload, then check to see if it returns properly with null. In Christ, Aaron Laws

[OriginalGriff]

I agree with the previous responses: readability is the important bit, and line spacing can seriously improve this - that is one reason why the "how to get an answer" bit at the top of the page stresses to use "code block" or <pre> tags to preserve formating. Three points: 1) Whatever the company style is, that is the one you use. Making it all look the same is the easiest way to make it all readable - because if the company style makes it all harder to read, then everyone will complain, and the company style will get changed. If everyone codes to a personal style, then everything gets muddled and harder to cope with. 2) My personal preference is always to use braces for a loop of conditional statement. That way, it is much harder to get it wrong when adding a second staement to a single line conditional. This is a lot easier nowadays, with VS etc auto indenting etc., but it is still easier to see what is going on. (It is similar to the reason why C++ accepts "if(A = B)" while C# wont - avoidance of potential problems.) 3) I also indent braces to match the statements in the functional block - I consider them to be a single indented statement in effect. I really hate the open brace at the end of an if statement, with it's matching brace indented. It looks clumsy and makes it difficult yo find matching braces by eye. But, if that is the company style, then I use it. They write the cheques...

No trees were harmed in the sending of this message; however, a significant number of electrons were slightly inconvenienced. This message is made of fully recyclable Zeros and Ones

[DaveyM69]

I meant within the class itself. Consider this int wrapper class (I've just written it out quickly so is very rough!) and compare the two different == operator overloads...

public class MyIntWrapperClass : IEquatable<MyIntWrapperClass>
{
private int _Value;
public MyIntWrapperClass(int value)
{
_Value = value;
}

// This method will throw a null reference exception if either parameter is null
/\* public static bool operator ==(MyIntWrapperClass a, MyIntWrapperClass b)
{
    return a.\_Value == b.\_Value;
}\*/

// This method works if either is null. If both are null, returns true.
public static bool operator ==(MyIntWrapperClass a, MyIntWrapperClass b)
{
    bool result = false;
    if (!ReferenceEquals(a, null))
    {
        if (!ReferenceEquals(b, null))
            result = a.\_Value == b.\_Value; // neither are null so compare \_Value
        else
            result = false; // b is null but a isn't
    }
    else
        result = ReferenceEquals(b, null); // true if both are null
    return result;
}
public static bool operator !=(MyIntWrapperClass a, MyIntWrapperClass b)
{
    return !(a == b);
}
public int Value
{
    get { return \_Value; }
}
public override bool Equals(object obj)
{
    return Equals(obj as MyIntWrapperClass);
}
public bool Equals(MyIntWrapperClass other)
{
    // using == overload
    return this == other;
}
public override int GetHashCode()
{
    return \_Value; ;
}
public override string ToString()
{
    return \_Value.ToString();
}

}

Dave
Generic BackgroundWorker - My latest article!
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)
Why are you using VB6? Do you hate yourself? (Christian Graus)

modified on Wednesday, October 7, 2009 5:24 PM

[PIEBALDconsult]

They're why I left Boston for California. :cool: (Not really.)

[PIEBALDconsult]

Hear hear!

OriginalGriff wrote:

I consider them to be a single indented statement in effect

They are one statement -- a compound statement. At least that's how it is in C/C++, it seems the C# guys didn't like that concept for some reason.