Condition outside loop or separate loop for different condition?

[-]

Should I adopt the first or the second way?
The first makes it faster. The second makes the code shorter.
The size to compare is not in the millions so which should i go with?

First Way:

private Comparator createXComparator() {
return new Comparator() {
public int compare(Object object1, Object object2) {
int value = 0;

A a1 = (A) object1;
A a2 = (A) object2;

if (a1.getX() < a2.getX()) {
value = -1;
} else {
value = 1;
}

return value;
}
};
}

private Comparator createYComparator() {
return new Comparator() {
public int compare(Object object1, Object object2) {
int value = 0;

A a1 = (A) object1;
A a2 = (A) object2;

if (a1.getY() < a2.getY()) {
value = -1;
} else {
value = 1;
}

return value;
}
};
}

Second Way:

private Comparator createComparator() {
return new Comparator() {
public int compare(Object object1, Object object2) {
int value = 0;

A a1 = (A) object1;
A a2 = (A) object2;

if (...) {
if (a1.getX() < a2.getX()) {
value = -1;
} else {
value = 1;
}
} else {
if (a1.getY() < a2.getY()) {
value = -1;
} else {
value = 1;
}
}

return value;
}
};
}

 

[Christian Kaufhold]

private Comparator createXComparator() {
return new Comparator() {
public int compare(Object object1, Object object2) {
int value = 0;

A a1 = (A) object1;
A a2 = (A) object2;

if (a1.getX() < a2.getX()) {
value = -1;
} else {
value = 1;
}

return value;

Note that this implementation is wrong (inconsistent results for
a1.getX() == a2.getX()). Also (in this case) there is no need to
create more than one XComparator as it has no state.



Christian

 

[Ingo R. Homann]

Hi -,

Should I adopt the first or the second way?
The first makes it faster. The second makes the code shorter.
The size to compare is not in the millions so which should i go with?

Why are you so sure that there are not millions of comparisons? Because
the Comparator is only used inside your package as an internal?

Anyway, I would always use the faster way. So, if you will reuse the
code in the future (which you probably are not planing now), it is safe
for using it in "millions-comparison-environments".

If you want so, you can add a convenience method:

Comparartor createComparator() {
if(...) {
return createXComparator();
} else {
return createYComparator();
}
}


Ciao,
Ingo

 

[-]

Note that this implementation is wrong (inconsistent results for
a1.getX() == a2.getX()).

To make it right is to synchronize it?

Also (in this case) there is no need to
create more than one XComparator as it has no state.

I don't get what you mean. Do rephrase.

Thanks.

 

[Christian Kaufhold]

To make it right is to synchronize it?

No, to return 0 then, as the state in question is equal.

I don't get what you mean. Do rephrase.

How does one XComparator differ from another?


Christian

 

[-]

No, to return 0 then, as the state in question is equal.

return 0 when a.getX() == a.getY() ?

How does one XComparator differ from another?

No difference. So I should make it static or something?

Perhaps you can correct the example earlier so that i can visually see
what's wrong

 

[-]

No, to return 0 then, as the state in question is equal.

Oh i get it

int value = 0;

if (a1.getX() < a2.getX()) {
value = -1;
} else if (a1.getX() > a2.getX()) {
value = 1;
}

return 0;

How does one XComparator differ from another?

I don't understand this though.

 

[Dale King]

Oh i get it

int value = 0;

if (a1.getX() < a2.getX()) {
value = -1;
} else if (a1.getX() > a2.getX()) {
value = 1;
}

return 0;

Note that Comparator does not require that the values be restricted to
-1, 0, and 1. You can use any negative number instead of -1 and any
positive number instead of 1. Therefore if getX returns an integer the
above can be replaced with:

return a1.getX() - a2.getX();

I don't understand this though.

I think he meant that there is no actual state information in the
comparator. So you only need one instance of the comparator to exist in
your entire program. You could create one and each call returns the same
instance.

 

[Chris Uppal]

Therefore if getX returns an integer the
above can be replaced with:

return a1.getX() - a2.getX();

Only if the range of getX() is small enough that integer wrap-around can be
guaranteed not to occur.

-- chris

 

[Eric Sosman]

Note that Comparator does not require that the values be restricted to
-1, 0, and 1. You can use any negative number instead of -1 and any
positive number instead of 1. Therefore if getX returns an integer the
above can be replaced with:

return a1.getX() - a2.getX();

Not always. For example, consider what happens if
a1.getX() returns zero and a2.getX() returns INT_MIN.
This substitution is appropriate only if you're 100%
sure the subtraction cannot produce "wraparound."

 

[Remon van Vliet]

Not always. For example, consider what happens if
a1.getX() returns zero and a2.getX() returns INT_MIN.
This substitution is appropriate only if you're 100%
sure the subtraction cannot produce "wraparound."

This is not correct, 0 - INT_MIN = INT_MIN, and as such, his test would
still work. Once a1.getX() becomes negative then it will wraparound though,
so the point is valid, the example given isnt.

That said, as of 5.0 i would've preffered an enum for the result type of
compare(). CompareResult.LESS, CompareResult.EQUAL, CompareResult.GREATER or
something. Bit more elegant and i feel new features of the language should
be used consistently in the APIs of said language.

Remon

 

[Eric Sosman]

This is not correct, 0 - INT_MIN = INT_MIN, and as such, his test would
still work. Once a1.getX() becomes negative then it will wraparound though,
so the point is valid, the example given isnt.

0 - INT_MIN is INT_MIN, as you say, but that result has
already suffered wraparound. Consider: INT_MIN is negative,
and subtracting a negative number from zero "mathematically"
should yield a positive result. But the "delivered" answer
is negative, showing that programming and mathematics have
parted company somewhere along the way ...

But don't take my word for it; try it for yourself:

int x = 0;
int y = Integer.MIN_VALUE;
System.out.println("x < y = " + (x < y));
System.out.println("x - y < 0 = " + (x - y < 0));

 

[Remon van Vliet]

0 - INT_MIN is INT_MIN, as you say, but that result has
already suffered wraparound. Consider: INT_MIN is negative,
and subtracting a negative number from zero "mathematically"
should yield a positive result. But the "delivered" answer
is negative, showing that programming and mathematics have
parted company somewhere along the way ...

But don't take my word for it; try it for yourself:

int x = 0;
int y = Integer.MIN_VALUE;
System.out.println("x < y = " + (x < y));
System.out.println("x - y < 0 = " + (x - y < 0));

Oh i did not know that, i missed the point you were making (my apologies). I
was consider value wraparound (i.e -1 minus MAX_INT is positive), not it's
sign partner, which i wasnt aware exists for the case where one of the
operands is 0.

Remon