Velocity Reviews > Java > Condition outside loop or separate loop for different condition?

# Condition outside loop or separate loop for different condition?

-
Guest
Posts: n/a

 06-13-2005
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
Guest
Posts: n/a

 06-13-2005
- <(E-Mail Removed)> wrote:

> 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
Guest
Posts: n/a

 06-13-2005
Hi -,

- wrote:
> 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

-
Guest
Posts: n/a

 06-13-2005
Christian Kaufhold wrote:

> 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
Guest
Posts: n/a

 06-13-2005
- <(E-Mail Removed)> wrote:
> Christian Kaufhold wrote:
>
>> Note that this implementation is wrong (inconsistent results for
>> a1.getX() == a2.getX()).

>
> To make it right is to synchronize it?

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

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

How does one XComparator differ from another?

Christian

-
Guest
Posts: n/a

 06-13-2005
Christian Kaufhold wrote:
>
> 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

-
Guest
Posts: n/a

 06-13-2005
Christian Kaufhold wrote:

> 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
Guest
Posts: n/a

 06-14-2005
- wrote:
> Christian Kaufhold wrote:
>
>> 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;

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

>> How does one XComparator differ from another?

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

--
Dale King

Chris Uppal
Guest
Posts: n/a

 06-14-2005
Dale King wrote:

> 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
Guest
Posts: n/a

 06-14-2005

Dale King wrote:
> - wrote:
>
>>Christian Kaufhold wrote:
>>
>>
>>>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;

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

--
http://www.velocityreviews.com/forums/(E-Mail Removed)