Is this a bug in ?: Operator, the addition does not perform as expected

  • Thread starter Thread starter R. Kaushik
  • Start date Start date
R

R. Kaushik

I am posting the code snippet here. It seems to me that the ?:
Operator is not behaving properly. It seems like it is a bug:

TimeSpan ts1, ts2;
float dayHours = 0;

ts1 = (some_datetime_variable_1);

ts2 = (some_datetime_variable_2);

// ****************** Working Code *************************
// This computes dayHours correctly
if(ts1.TotalHours > 0)
dayHours = (float)ts1.TotalHours;

if(ts2.TotalHours > 0)
dayHours += (float)ts2.TotalHours;
// ****************** Working Code *************************

// ****************** Non-Working Code *************************
// This does not add the portion after + sign
dayHours = (float)(ts1.TotalHours > 0 ? ts1.TotalHours : 0 +
ts2.TotalHours > 0 ? ts2.TotalHours : 0);
// ****************** Non-Working Code *************************

As commented above, I am trying to condense the working code to one
line using ? operator but the one line non-working code dows not add
the variable after + sign. For example, if ts1.TotalHours is 4 and
ts2.TotalHours is 3, the dayHours is computed as 4. Though, I can use
the working code, which takes four lines but it is something I am
doing wrong here or it is a bug in .NET Framework?

Thank you for your time for looking at my post and would appreciate
your responses.

R. Kaushik
 
R. Kaushik said:
I am posting the code snippet here. It seems to me that the ?:
Operator is not behaving properly. It seems like it is a bug:

TimeSpan ts1, ts2;
float dayHours = 0;

ts1 = (some_datetime_variable_1);

ts2 = (some_datetime_variable_2);

// ****************** Working Code *************************
// This computes dayHours correctly
if(ts1.TotalHours > 0)
dayHours = (float)ts1.TotalHours;

if(ts2.TotalHours > 0)
dayHours += (float)ts2.TotalHours;
// ****************** Working Code *************************

// ****************** Non-Working Code *************************
// This does not add the portion after + sign
dayHours = (float)(ts1.TotalHours > 0 ? ts1.TotalHours : 0 +
ts2.TotalHours > 0 ? ts2.TotalHours : 0);
// ****************** Non-Working Code *************************

As commented above, I am trying to condense the working code to one
line using ? operator but the one line non-working code dows not add
the variable after + sign. For example, if ts1.TotalHours is 4 and
ts2.TotalHours is 3, the dayHours is computed as 4. Though, I can use
the working code, which takes four lines but it is something I am
doing wrong here or it is a bug in .NET Framework?

Thank you for your time for looking at my post and would appreciate
your responses.
Click to expand...

I don't think there's a bug in .NET, I think there's a problem with
your code in terms of lack of bracketting. I believe your code is being
compiled as:

dayHours = (float)(ts1.TotalHours > 0 ? ts1.TotalHours :
(0 + (ts2.TotalHours > 0 ? ts2.TotalHours : 0));

Instead, you want:

dayHours = (float)((ts1.TotalHours > 0 ? ts1.TotalHours : 0) +
(ts2.TotalHours > 0 ? ts2.TotalHours : 0));

However, this just demonstrates that the version of the code using the
conditional operator is harder to read than the longer version. Code
which takes four lines instead of one isn't necessarily bad - in fact,
it will often be better, as it's likely to be more readable (as in this
case).
 
R. Kaushik said:
[...]
As commented above, I am trying to condense the working code to one
line using ? operator but the one line non-working code dows not add
the variable after + sign.
[...]
Click to expand...

As Jon said, it's probably best to write out the fuller, more-readable
lines.

Personally, I might make a method called something along the lines of
GetDayHours() or CalculateDayHours(), which contains these lines. It is
then just a case of:

dayHours = GetDayHours(param1, param2, ..., paramN);

Alternatively, you could make a general-purpose static method called
EnsureNonNegative(), e.g.:

private static int EnsureNonNegative(int val)
{
return val >= 0 ? val : 0;
}

....and then say:

dayHours = (float)(EnsureNonNegative(ts1.TotalHours) +
EnsureNonNegative(ts2.TotalHours));

Just my £0.02.
 
Thanks Jon and C# Learner,

Bracketing saved the day for me. I thought about using bracket earlier
but thought that ? operator would take precedence. Obviously that was
not the case. I agree with your advice on readability but there are
times that the code file becomes so big, I become more inclined to
using condensed code like above. But that only helps reduce the file
size and takes so much time and effort while working through. Thanks
guys.

R. Kaushik
 
R. Kaushik said:
Bracketing saved the day for me. I thought about using bracket earlier
but thought that ? operator would take precedence. Obviously that was
not the case. I agree with your advice on readability but there are
times that the code file becomes so big, I become more inclined to
using condensed code like above. But that only helps reduce the file
size and takes so much time and effort while working through. Thanks
guys.
Click to expand...

I've never come across a situation where source file size is actually
more important than readability. I would *always* use the expanded
version in this case. There are times when it's worth using the
conditional operator, but I can't remember ever using it twice in the
same statement - readability just goes out the window.
 
Greetings

I understand about large code files. With VS studio .Net I am learning new techniques about writing code.

For example, I used to break up long sequences of code into separate functions or methods, even though these methods were not used else where. Now I use the #region.

With #region, I can create many different views of my code.

For example,

In R. Kaushik post, he was trying to do one long statement to reduce the code file size. Now, if this piece of code is
only used by one method, I would code it like this:

#region Compute Day Hours
if(ts1.TotalHours > 0
dayHours = (float)ts1.TotalHours

if(ts2.TotalHours > 0
dayHours += (float)ts2.TotalHours
#endregion

This way I can collaspe the outline in my method to get a summary of the operations, and I can expand a region when I want to see the operation detail.

This method should not be used a substitution for good refactoring. If the operation is performed is several places, it should be put in its own method.

Cheers

John



----- R. Kaushik wrote: ----

Thanks Jon and C# Learner

Bracketing saved the day for me. I thought about using bracket earlie
but thought that ? operator would take precedence. Obviously that wa
not the case. I agree with your advice on readability but there ar
times that the code file becomes so big, I become more inclined t
using condensed code like above. But that only helps reduce the fil
size and takes so much time and effort while working through. Thank
guys

R. Kaushi
 
John Morrill said:
I understand about large code files. With VS studio .Net I am
learning new techniques about writing code.

For example, I used to break up long sequences of code into separate
functions or methods, even though these methods were not used else
where. Now I use the #region.
Click to expand...

I still don't think that's a good idea though - I believe it's
fundamentally a better idea to break up long methods into smaller ones,
which can more easily be unit tested and debugged.

Just being able to collapse long methods so they're not as obviously
long doesn't mean that long methods are a good idea, IMO.
 
John,
In addition to Jon's comments about breaking long methods into smaller ones.

I find its "equally" import to break long classes into smaller ones.

Martin Fowler's book "Refactoring - Improving the Design of Existing Code"
from Addison Wesley http://www.refactoring.com/ provides a number of
Refactorings on how to improve the design of existing code, plus the reasons
you would want to.

Hope this helps
Jay

John Morrill said:
Greetings!

I understand about large code files. With VS studio .Net I am learning new techniques about writing code.

For example, I used to break up long sequences of code into separate
Click to expand...
functions or methods, even though these methods were not used else where.
Now I use the #region.
With #region, I can create many different views of my code.

For example,

In R. Kaushik post, he was trying to do one long statement to reduce the
Click to expand...
code file size. Now, if this piece of code is
only used by one method, I would code it like this:


#region Compute Day Hours
if(ts1.TotalHours > 0)
dayHours = (float)ts1.TotalHours;

if(ts2.TotalHours > 0)
dayHours += (float)ts2.TotalHours;
#endregion

This way I can collaspe the outline in my method to get a summary of the
Click to expand...
operations, and I can expand a region when I want to see the operation
detail.
This method should not be used a substitution for good refactoring. If the
Click to expand...
operation is performed is several places, it should be put in its own
method.
 
R. Kaushik,
In addition to Jon's comments writing readable code.

Martin Fowler's book "Refactoring - Improving the Design of Existing Code"
from Addison Wesley http://www.refactoring.com/ provides a number of
Refactorings on how to improve the design of existing code, plus the reasons
you would want to.

Hope this helps
Jay
 
Back
Top