ncredinburgh / JavaForSmallTeams

Guidance on writing "good" server side java
https://www.gitbook.com/book/ncrcoe/java-for-small-teams
Other
69 stars 31 forks source link

Tweaks, grammar and whitespace #29

Closed stoopbrain closed 8 years ago

stoopbrain commented 8 years ago

Hi again, read a few more chapters this evening 👍 . These two changes are semantic differences: 5fca6c2cf03e4853722407f541b3aca78ea5c277 (Reorder code example according to logical flow) 90237faa0496af5dacbfd4dc2b0bf275fb149b87 (Semantic change: Adding a "not")

The rest are grammar and whitespace fixes (might be worth removing trailing whitespace from all files in one go?)

Let me know if you'd prefer any/all of these squashed! Cheers

hcoles commented 8 years ago

Thanks - I do seem to like apostrophes in "it's".

The additional of "not" in

https://github.com/NCR-CoDE/JavaForSmallTeams/commit/90237faa0496af5dacbfd4dc2b0bf275fb149b87

Doesn't make sense, which I suspect means that what the sentence is trying to communicate is not clear.

What it's meant to say is you should only consider using the reflection based implementations if you are confident that you have good performance tests and regularly profile your code to identify which (if any) of the equals/hashcode methods have a meaningful performance impact on your application.

Can you suggest an alternate wording that would be clearer?

stoopbrain commented 8 years ago

Ah yes - sorry just me misinterpreting. I'll revert that commit from this PR.

Here are a few suggestions for alternate wording:

Option 1) Replace "pick up" with "detect" The brevity of these implementations is attractive, but their performance is measurably poor compared with all the implementations discussed so far. If you are confident that you would detect real performance bottlenecks with testing and profiling then using these as initial placeholder implementations may be a reasonable approach, but in general we suggest you avoid them.

Option 2) Terse The brevity of these implementations is attractive, but their performance is measurably poorer than others discussed so far. You should only consider using these as initial placeholder implementations if you are confident that your performance testing and profiling activities would detect any resulting performance bottlenecks in your application.

Option 3) Expanded The brevity of these implementations is attractive, but their performance is measurably poorer than others discussed so far. Good performance tests and regular profiling can help determine whether a poorly performing method genuinely leads to performance bottlenecks in your application. If you are confident that you would detect such adverse impacts then using these methods as initial placeholder implementations may be a reasonable approach. But in general we suggest you avoid them.

hcoles commented 8 years ago

Thanks, I think option 3 is a good improvement - if you raise it as a separate PR I'll merge it in.

stoopbrain commented 8 years ago

Thanks! #30