المساعد الشخصي الرقمي

مشاهدة النسخة كاملة : Code Readability Poll



C# Programming
10-08-2009, 06:00 AM
Dear Sirs,

I just came across some code and changed it. I'm implementing the java Gridbag layout manager to a panel in C#, and in the Insets.Equals(object) code, the following could be found:

if (obj instanceof Insets) {
Insets insets = (Insets)obj;
return ((top == insets.top) && (left == insets.left) &&
(bottom == insets.bottom) && (right == insets.right));
}
return false;
So I sharpified it:
if (obj is Insets)
{
Insets insets = obj as Insets;
return ((top == insets.top) && (left == insets.left) && (bottom == insets.bottom) && (right == insets.right));
}
return false;
Then, almost compulsively, I modified it thus:
Insets insets = obj as Insets;
if (ReferenceEquals(insets, null)) return false;
return ((top == insets.top) && (left == insets.left) && (bottom == insets.bottom) && (right == insets.right));
I do this kind of thing ALL the time. I would like some comments on readability and efficiency.

I guess to characterize the modification, I would say that I make it read more sequentially, without having to skip code (casting your eyes over code because in certain cases it would not be executed). Also, I'm crazy about conserving lines. I suppose that's because I'm not paid per line http://www.barakasoft.com/script/Forums/Images/smiley_wink.gif . I always use the same-line `if' if I can, and if not, I resort to the no-brace 'if'. Which sometimes means...oh yeah, let me show a line I wrote the other day. Here's what I first wrote:
foreach (Point a in _gr_pts)
{
if (prev == Point.Empty || prev == a)
{
prev = a;
continue;
}
g.DrawArc(_ap_pen, prev, a);
prev = a;
}

Then, I changed it to:
foreach (Point a in _gr_pts)
{
if (prev == Point.Empty || prev == a)
{
prev =a;
continue;
}
//Crazy, I know.
g.DrawLine(_ap_pen, prev, prev = a);
}
I tried for a while to figure out how to get the braces out of the if statement...oh yeah, I just realized that this is possible:
(I'm writing free-hand, so there may be typos...)
foreach (Point a in _gr_pts)
if (prev == Point.Empty || prev == a)
{
prev = a;
continue;
}
else g.DrawLine(_ap_pen, prev, prev = a);

I know some people swear by ALWAYS using braces in if, else, do, while, for, foreach statements, (and if I remember correctly, Visual studio has some option about including those compulsively) and I am a little curious if any of you are that way, or how many would do what I do.

To sum up, some items on which to comment:
I cast the object first without asking about the cast. I use ReferenceEquals, not `=='. I return false inline with the if-statement; I don't use else.

When I say things like `I don't use else,' I don't mean all the time, but I don't use it if I don't need it. Does anyone know if this affects effeciency in any way? I suppose I'm too lazy to check out the disassembly (because it's always SOO confusinghttp://www.barakasoft.com/script/Forums/Images/smiley_frown.gif ).

I look forward to hearing back from you, my esteemed colleagues.

In Christ,
Aaron Laws