2

I have this code

Instant now = Instant.now();
if (amountDays >= 0) {
    now = now.plus(amountDays, ChronoUnit.DAYS);
} else {
    now = now.minus(Math.abs(amountDays), ChronoUnit.DAYS);
}

And I was thinking to simplify it like this

Instant now = Instant.now();
now = now.plus(amountDays, ChronoUnit.DAYS);

However, I am unsure if plus works correctly with negative values or if that messes up the result.

Can I use plus like that, with possibly negative values?

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
KunLun
  • 3,109
  • 3
  • 18
  • 65
  • In general you should of course prefer `minus` if you want to go back in time. Readability matters. In this particular case however, I would stick with just `plus` and maybe add a comment `// amountDays can also be negative` or just document it nicely in your javadoc. Voting to close this as it is opinion-based. – Zabuzard Feb 10 '21 at 12:57
  • 2
    Note the javadoc of `plus` which says _"amountToAdd - the amount of the unit to add to the result, may be negative"_ - so it is supported and also expected that this method can be used like that. – Zabuzard Feb 10 '21 at 12:59
  • 1
    @Zabuzard, I ask that because I am curious if something wrong can happen at execution, more than `opinion-based`. – KunLun Feb 10 '21 at 13:01
  • Then the answer is **no**, it will work fine. The method, according to its javadoc, supports this use case. – Zabuzard Feb 10 '21 at 13:01
  • 3
    [`minus` delegates to `plus`](https://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/java/time/Instant.java#l982), so it shouldn't make much of a difference. – TiiJ7 Feb 10 '21 at 13:01
  • 1
    This is purely "style", no technical difference. Given the fact that "minus" just uses "plus", go for the 2nd, simple solution that requires way less code. – GhostCat Feb 10 '21 at 13:03

2 Answers2

4

plus with negative values

The plus method supports adding negative times to go back in time, from its documentation:

amountToAdd - the amount of the unit to add to the result, may be negative

So all good, you can use it like that and it will work as expected.

Implementation

Little trivia, the current implementation of minus even delegates to plus with -amountToSubtract as value:

return (amountToSubtract == Long.MIN_VALUE
    ? plus(Long.MAX_VALUE, unit).plus(1, unit)
    : plus(-amountToSubtract, unit));

Notes

In general, if you just want to go back in time, prefer using minus for readability.

In your particular case I would stick with just plus though to not bloat the code and logic unecessarily. Instead, prefer adding a comment

// amountDays may be negative

or ensure that your javadoc is clear about that.

Minor improvement, you can simplify your code from two statements to just one:

Instant now = Instant.now().plus(amountDays, ChronoUnit.DAYS);
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Zabuzard
  • 25,064
  • 8
  • 58
  • 82
  • 1
    Of course "now" is not a good name for the variable once you've offset the value by days, unless you've invented a time machine and go with it. – Andreas Feb 10 '21 at 13:48
2

Just take a look how minus is implemented

@Override
public Instant minus(long amountToSubtract, TemporalUnit unit) {
    return (amountToSubtract == Long.MIN_VALUE ? plus(Long.MAX_VALUE, unit).plus(1, unit) : plus(-amountToSubtract, unit));
}

Of course for readability sake it makes sense to use both plus and minus in proper situations but checking whether amountDays is greater or lower from 0 seems to be some kind of internal Instant.plus logic and is deifinitely not helping with readability of your code

m.antkowicz
  • 13,268
  • 18
  • 37