Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/334
Thanks @PascalSchumacher for confirming it's actually backward compatible
(#TodayILearned - and hopefully will remember it).
@ingvarc , I agree on @aaabramov regarding the `+1`. When I read it, I
first thought that was a `+= 1`, and then re-read it and understood his
concern. Might be worth simplifying it. If you'd like to keep the commits
simpler (which is great initiative, thanks!) you could try rebasing commits and
push+force to your branch.
Good points @stokito. Not sure about branch prediction after the spectre
bug. I think one of the mitigations involved disabling branch prediction.
Though I think micro-benchmarking isn't priority in lang.
>Here developer sees a "happy path" first and only then he or she sees
exception situation handling.
This is more natural to understand. As I remember this style was mentioned
in Complete Code book.
Code Complete is one of my favourite books. For me, that code could be:
```java
private static void addProcessor(final String key, final Processor
processor) {
if (ARCH_TO_PROCESSOR.containsKey(key)) {
throw new IllegalStateException("Key " + key + " already exists
in processor map");
}
ARCH_TO_PROCESSOR.put(key, processor);
}
```
I think maybe developers would read the first `if` as a validation block.
Then the normal behaviour, without the else (I think omitting the else in cases
like this is also in Code Complete... or Pragmatic Programmer? Can't recall).
I think the other changes like removing class name, inverting some `if`
conditions, and removing the exceptions not thrown look good, and the code
readability keeps OK. Great contribution @ingvarc , thanks!
---
