4

Say my comparator function foo takes a time sequence as input, which could be a series of time from small to large, or from large to small.

Foo may look like this:

Foo(List<double> timestampList) {
  if (currentTime > previousMaxValueTimestamp) {
    ...
  } else if (curremtTime > previousMinValueTimestamp) {
    ...
  }
}

The above works for forward sequence, but not the reverse one. How can I elegantly write a logic that works for both types of sequence? Below is what I want to do, but it duplicates most of the code, which is not desired.

Foo(List<double> timestampList, boolean isForward) {
  if (isForward) {
        if (currentTime > previousMaxValueTimestamp) {
            ...
        } else if (curremtTime > previousMinValueTimestamp) {
            ...
        }
  } else {
        if (currentTime < previousMaxValueTimestamp) {
            ...
        } else if (curremtTime < previousMinValueTimestamp) {
            ...
        }
  }
}

My current solution is like below. Is it good coding style?

Foo(List<double> timestampList, boolean isForward) {
    if ((isForward && currentTime > previousMaxValueTimestamp) || (!isForward && currentTime < previousMaxValueTimestamp)) {
        ...
    } else if ((isForward && curremtTime < previousMinValueTimestamp) || (!isForward && currentTime > previousMaxValueTimestamp)) {
        ...
    }
}
Stan
  • 37,207
  • 50
  • 124
  • 185
  • are all variables here Timestamps ? currentTime etc ? Because then you can simply use the methods from Timestamp, such as after or before (see https://docs.oracle.com/javase/8/docs/api/java/sql/Timestamp.html) – Emerson Cod Nov 10 '15 at 07:36
  • It's our self-defined timestamp, "double" typed. Not the general Timestamp. – Stan Nov 10 '15 at 07:38
  • Would sorting the list work? – npinti Nov 10 '15 at 07:41
  • This is critical section, we don't want extra overhead to sort or process the timestamp list. – Stan Nov 10 '15 at 07:42
  • 1
    based on your current solution I would suggest to extract the conditions in methods to give a meaningful name. This will make reading and understanding of the solution a lot easier – Emerson Cod Nov 10 '15 at 07:45

3 Answers3

6

I don't know if something like this will still satisfy you, but in this way you can still reduce more or less your structure and you will not get this inflated code

if (isForward ? currentTime > previousMaxValueTimestamp :
    currentTime < previousMaxValueTimestamp)
{

} else if (!isForward ? currentTime < previousMaxValueTimestamp :
           currentTime < previousMinValueTimestamp)
{

}
randers
  • 5,031
  • 5
  • 37
  • 64
Lukas Hieronimus Adler
  • 1,063
  • 1
  • 17
  • 44
1

If you have the list sorted as i suspect you do, you can simply use an iterator :

ListIterator li = a.listIterator(a.size());
while(isForward ? li.hasPrevious() : li.hasNext()) {
  nextItem = isForward ? li.previous() : li.next());
  //do stuff

}

taken from here and then edited to fit your needs

if your items are not pre-sorted and you can live with the pre-sorting overhead, you may use Collections.sort(yourList,yourSpecialComparator) providing your own special comparator.

Community
  • 1
  • 1
Ofek Ron
  • 8,354
  • 13
  • 55
  • 103
  • In my case, the comparing logic is more complicated. Traversing reversely doesn't really help. Thanks though. – Stan Nov 10 '15 at 08:50
  • 1
    You can sort your list using Collections.sort(yourList,yourSpecialComparator) and the use the suggested approach. – Ofek Ron Nov 10 '15 at 08:53
1

For simplicitly, have one "normal" comparator, and use .reverse() for descending lists. You can of course abstract it in a method if needed.

Viktor Mellgren
  • 4,318
  • 3
  • 42
  • 75