fix some typos and other small refactorings

[DarioViva42]

• fix some typos and rephrase some sentences to make their meaning clearer
• use String.isEmpty and replace null check plus empty check with already existing StringUtilities.isEmpty
• replace some null safe equals with NullSafe.equals
• use advanced for loop
• use a little bit more generics
• replace some string concatenations with StringBuilder
• remove some unnecessary null checks
• simplify some boolean expressions
• change some access modifiers
• update README to a more formal tone

[kwwall]

@DarioViva42 - Okay. Please understand one thing. ESAPI 2.x is basically in "maintenance mode" and by that I mean that we do not generally intend to do:

1. Development of new features except unless them bulk of the changes are simple and done my someone else.
2. Minor cosmetic changes.

There is a reason for that. The 3 of us (myself, @xeno6696, and @jeremiahjstacey ) really would like to get on to working on ESAPI 3.0 and these minor, on-going tasks keep distracting us from that bigger picture. While we are willing to merge the types of commits that that you are doing, I think this has grown beyond "fix some small typos and other small refactoring". Sure, each individual commit is small and for the most part trivial. But there are a lot of them; so far, 57 of them and across 84 files to boot, if I've added them all up correctly. The problem is they add up in time it takes us to review this. We have a policy of doing code reviews of all the commits and for someone submitting PRs outside of the core ESAPI team, we require at least 2 of us to review it. And when it's as broad as this, because we are not all experts across the entire code base, a lot of time broad changes like this ends up requiring approvals from all 3 of us. Except as all also have full-time day jobs and families and other interests. So, when you to a single PR this complex and broad, getting the code reviews done is likely to take a really long time. (We've been going through code review of other PRs for a long time now.)

So, I am going to reject this PR and make a suggestion that will help us and I think ultimately you as well. I'm a firm believer in not letting the perfect become the enemy of the good. So I would like you to cancel this PR and create (at least 3) smaller PRs. For example, you might approach it with something like this:

• One PR to only correct typos in public documentation (so .md files, comments in .properties / configuration files, and Javadoc.
• Changes to made to code that is restructured because is it is, it can potentially result in an error in some unusual edge cases. (E.g., commit #19ac3d7)
• Changes to code that is cosmetic in nature (and only if the changes are simple / obvious); examples would be cases of replacing a while-loop with an equivalent for-loop.
• Non-Javadoc comments in the rest of the Java code. (To me these are really low priority. I see there's a lot of them and in general, they really are not important in the grand scheme of things unless they potentially cause confusion to someone reading through the code.)

Every PR that you submit needs to have a corresponding GitHub issue created for it and the PR should reference that issue. Also, you need not create all the PRs at once, but if you do, I think we would likely address them in the order that I noted above.

I know from 15 years of working on ESAPI, if we don't take an approach like this, that this PR is just going to sit there for a year until it finally gets merged because we simply can't focus on this many changes at once, especially if you are continuing to make more commits as you find them. But this PR just will not work for us. We need bite-sized PRs that we can review in a couple hours on a weekend. So break it down into significantly smaller PRs. Keep in mind our reviews also often provide extensive feedback and requests to change things and that's a major reason why large, broad PRs don't work well. Please resubmit these as multiple PRs that are each more focused. If you don't, it will likely be a year or so, when we finally get this approved.

Thanks for your understanding in this matter.

[DarioViva42]

I absolutely understand. I think it would help, if this would be noted in the READMDE.md though.

Sure, each individual commit is small and for the most part trivial. But there are a lot of them; so far, 57 of them and across 84 files to boot, if I've added them all up correctly.

No, you have not, there are 172 commits. 😋 Not that this would change your mind of merging the PR. 😁

By the way, you do not need to count commits, you can see the number of commits in the PR header.

Also, you need not create all the PRs at once

How would I even create three PRs at once? I only know of a way to create them one after another.

[kwwall]

@DarioViva42 asked:

How would I even create three PRs at once? I only know of a way to create them one after another.

I really meant, you needn't submit all the PRs at once, but can space them out. Because even if you drop all 174 of those commits in 3 or 4 smaller PRs it's still going to take us a while to work through them. I definitely would recommend keeping track of them in separate branches though. (See CONTRIBUTING-TO-ESAPI.txt.)

[DarioViva42]

@kwwall

I really meant, you needn't submit all the PRs at once, but can space them out.

I was going to do that anyway ;) got sick and now I feel like a 100 year old man, so even if you, all of a sudden, want me to rush all the PRs, I would not be able to do that.

I definitely would recommend keeping track of them in separate branches though. (See CONTRIBUTING-TO-ESAPI.txt.)

Of course! Every PR needs its own branch. I think github would not even allow to break this rule.

[kwwall]

Sorry that you are sick. And, you could do it all on your same branch, if we did it serially. That is, you do the commits, push to your fork, create a PR, we review the PR, you sync your changes with ESAPI/esapi-java-legacy develop branch, and then lather, rinse, repeat.

[kwwall]

Also, I'm going to rewrite CONTRIBUTING-to-ESAPI as markdown and then probably make CONTRIBUTING-to-ESAPI.txt a symlink to CONTRIBUTING-to-ESAPI.md, so when I do that, I will modify that to mention keeping the PRs small.

[DarioViva42]

And, you could do it all on your same branch, if we did it serially.

Oh right, I guess that would work. But no, that is not what I am going to do, as you said, keeping separate branches is the cleaner way.

Cherry-Picking the commits is easy enough.