Can this code be written in a shorter way

[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

 

[Des]

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;
}
}

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)

 

[Marcel Müller]

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

Shorter for sure. But what are you trying to accomplish?

int IComparer<PointF>.Compare(PointF point1, PointF point2)

{
return currentMode == Mode.X
? point1.X.CompareTo(point2.X)
: point1.Y.CompareTo(point2.Y);
}


Marcel

 

[Marcel Müller]

Math.Sign(point1.X - point2.X)

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


Marcel

 

[Marcel Müller]

Hallo,

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.

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

When you want a comparison, _write a comparison_.

Anyway, indeed.


Marcel

 

[Jeff Johnson]

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;
}
}

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.

 

[Tony Johansson]

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.

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

 

[Jeff Johnson]

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);
}

I'll take simple code that doesn't use method calls over simple code that
does any day.

 

[Des]

[...]

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

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

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

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.

 

[Des]

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


Marcel

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).