Can this code be written in a shorter way

  • Thread starter Thread starter Tony Johansson
  • Start date Start date
T

Tony Johansson

Here is a working code but I just wonder if it's possible to write the code
so the amount of code is shorter ?

int IComparer<PointF>.Compare(PointF point1, PointF point2)
{
if (currentMode == Mode.X)
{
if (point1.X > point2.X)
return 1;
else if (point1.X < point2.X)
return -1;
else
return 0;
}
else
{
if (point1.Y > point2.Y)
return 1;
else if (point1.Y < point2.Y)
return -1;
else
return 0;
}
}

//Tony
 
Here is a working code but I just wonder if it's possible to write the code
so the amount of code is shorter ?

int IComparer<PointF>.Compare(PointF point1, PointF point2)
{
if (currentMode == Mode.X)
{
if (point1.X> point2.X)
return 1;
else if (point1.X< point2.X)
return -1;
else
return 0;
}
else
{
if (point1.Y> point2.Y)
return 1;
else if (point1.Y< point2.Y)
return -1;
else
return 0;
}
}
Click to expand...

I think it could be written in 1 line using a conditional operator, if
you wish.
Below is a pointer.

Math.Sign(point1.X - point2.X)
 
Tony said:
Here is a working code but I just wonder if it's possible to write the code
so the amount of code is shorter ?
Click to expand...

Shorter for sure. But what are you trying to accomplish?
int IComparer<PointF>.Compare(PointF point1, PointF point2)
Click to expand...
{
return currentMode == Mode.X
? point1.X.CompareTo(point2.X)
: point1.Y.CompareTo(point2.Y);
}


Marcel
 
Des said:
Math.Sign(point1.X - point2.X)
Click to expand...

Note that this will throw an exception for singular values like NaN. A
comparer normally should not throw.


Marcel
 
Hallo,

Peter said:
It will also return the wrong value for certain inputs. For example:

Point p1 = new Point(int.MinValue, 0),
p2 = new Point(1, 0);

p1 should compare as less than p2, but the "clever" approach will claim
that p2 is greater than.
Click to expand...

the OP used floting point values (PointF). So this will not happen.

When you want a comparison, _write a comparison_.
Click to expand...

Anyway, indeed.


Marcel
 
Here is a working code but I just wonder if it's possible to write the
code so the amount of code is shorter ?

int IComparer<PointF>.Compare(PointF point1, PointF point2)
{
if (currentMode == Mode.X)
{
if (point1.X > point2.X)
return 1;
else if (point1.X < point2.X)
return -1;
else
return 0;
}
else
{
if (point1.Y > point2.Y)
return 1;
else if (point1.Y < point2.Y)
return -1;
else
return 0;
}
}
Click to expand...

Yes, but any kind of shortening will result in pretty much the same
generated IL and will make the code less clear. The fact is that you have
hit on something so fundamental that you really can't simplify it further. I
know the general gut reaction is "There's got to be another way," but if you
think about it, eventually you can only get code simplified so far until you
hit "the basics," and that's what you've got right there: if statements and
returns. Basic, basic, basic.
 
Jeff Johnson said:
Yes, but any kind of shortening will result in pretty much the same
generated IL and will make the code less clear. The fact is that you have
hit on something so fundamental that you really can't simplify it further.
I know the general gut reaction is "There's got to be another way," but if
you think about it, eventually you can only get code simplified so far
until you hit "the basics," and that's what you've got right there: if
statements and returns. Basic, basic, basic.
Click to expand...

This code is much shorter. I mean the generated IL code might not be shorter
but the main point is
that the code that I write is short.
This code is also easy to understand.
int IComparer<PointF>.Compare(PointF point1, PointF point2)
{
return currentMode == Mode.X
? point1.X.CompareTo(point2.X)
: point1.Y.CompareTo(point2.Y);
}

//Tony
 
This code is much shorter. I mean the generated IL code might not be
shorter but the main point is
that the code that I write is short.
This code is also easy to understand.
int IComparer<PointF>.Compare(PointF point1, PointF point2)
{
return currentMode == Mode.X
? point1.X.CompareTo(point2.X)
: point1.Y.CompareTo(point2.Y);
}
Click to expand...

I'll take simple code that doesn't use method calls over simple code that
does any day.
 
[...]
p1 should compare as less than p2, but the "clever" approach will
claim that p2 is greater than.
Click to expand...

the OP used floting point values (PointF). So this will not happen.
Click to expand...

Ah, yes. Should have looked more closely. Thanks.

Even so, other bad things can happen with overflowed values (such as the
exception you mentioned…not sure why I didn't put 2 and 2 together
there; tunnel-vision, I guess).

Subtractions aren't comparisons; they should only be used as such if
there is both a VERY good reason for doing so (e.g. the values have to
be subtracted anyway and performance is for some reason _critical_),
_and_ one can guarantee the values stay within a small range that
guarantees the result of the subtraction remains valid for comparison
purposes.

It is exceedingly rare for both of those conditions to hold. Heck, the
first condition alone is exceedingly rare.

Even ignoring the correctness issue, the other big problem with being
"clever" like that is that it makes the code much harder to understand.
If you write a comparison, it's obvious you're comparing things. If you
try to implement a comparison using something other than a comparison,
now every time someone looks at that code, they have to figure out
whether the "other than a comparison" code is actually performing a
comparison.

Just say "no". :)

Pete
Click to expand...

Thanks guys for the input. I'm new to c# and didn't even know there was
a CompareTo.

Not so sure of your 'cleverness' jibe though.
 
Note that this will throw an exception for singular values like NaN. A
comparer normally should not throw.


Marcel
Click to expand...

Ok, I am just learning c#, just finding my way around vs.
So what would I expect the comparer to return, NaN makes sense to me.
Surely if it returns NaN it is different to the OPs code (although what
I expect he wanted).
 
Back
Top