Refactoring and formatting of the codebase

[douglasrizzo]

This PR provides a refactored and reformatted code base. Formatting was done following the Google Java Style Guide, especifically a Google-provided configuration file for Intellij Idea. The Google style was changed for 4 identation spaces instead of 2.

A .editorconfig file is provided to enforce the style in compatible editors and IDEs.

As for the refactoring, all fixes are separated in individual commits in order to be better reviewed. I have only changed what did not seem to serve an explicit purpose. Some pieces of code seemed to exist intentionally (empty if and else statements, ArrayList objects that are populated but never used). In these cases, I decided to not change what was present.

I made sure to keep the code compatible with the oldest possible Java version (8 or 9).

The code still compiles and runs.

Here is a list of what I considered relevant to refactor:

• use addAll, fill, clear, arraycopy and data structure constructors to fill the structures in bulk, avoiding lots of loops;
• use StringBuilder where many string concatenations occur, such as when creating JSON and XML content;
• early stopped loops (using break), where possible;
• simplified boolean expressions in if, return and switch statements;
• changed println("") to println();
• declared arrays in Java-style (int[] x), instead of C-style (int x[]);
• started accessing static attributes using class instead of object instance;
• added comments pointing to lists that are populated, but never used;
• rearranged methods (@Overrides come first, then methods are list by usage, according to a breadth-first search);
• removed unnecessary variable initialization;
• removed unused or too broad imports;
• removed unused variables;
• removed unnecessary boxing of variables (e.g. new Integer(1));
• removed unnecessary and duplicate semi-colons;
• removed unneeded qualifiers from interfaces and abstract classes (like public and abstract);
• removed unnecessary castings;
• removed unnecessary exceptions from function throws.

[santiontanon]

Wow, this is a very large change list! it will take me a long time to go through these, since MANY files are changed... hahaha. So, please give me time for this! ;)

Also, btw, some of those commits are great, but a few others I would like for them to remain as they currently are. Is there a way to selectively choose which of these to accept?

[douglasrizzo]

You can pull my douglasrizzo:refactor to another branch of your repo, then selectively apply my commits to your master branch using git cherry-pick.

This is independent of this pull request, though. I don't think you can selectively accept commits through there.

If you show me which commits contain undesired changes, I can try to create a new branch (and pull request) with only the changes that would be accepted to master.

[santiontanon]

Thanks a lot Douglas!! Some of these commits are great contributions, so, thank you very much! I'll prepare a list of the commits that I'm ready to accept when I get back home from work later today and get back to you! :)

…

On Mon, Nov 18, 2019 at 9:56 AM Douglas De Rizzo Meneghetti < ***@***.***> wrote: You can pull my douglasrizzo:refactor to another branch of your repo, then selectively apply my commits to your master branch using git cherry-pick <https://git-scm.com/docs/git-cherry-pick>. This is independent of this pull request, though. I don't think you can selectively accept commits through there. If you show me which commits contain undesired changes, I can try to create a new branch (and pull request) with only the changes that would be accepted to master. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#54>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIEJPOAU4LCZAMQ4ZYLNVLQULJMPANCNFSM4JOEAVEQ> .

[santiontanon]

Hi Douglas! I have finally had time to look through this pull request in detail. I think the best would be to start with a few of these commits, and then we can look at the other ones bit by bit. So, I am going to reject the current pull request. But if you could do a pull request that includes these commits, I'll accept right away:

• remove unnecessary variable initialization
• removed unused imports
• using StringBuilder where there are many string concatenations
• interfaces and abstract classes don't need these qualifiers
• no need for boxing
• early stopping loops
• ? already extends Object
• simpler way to find max value of array
• using standard charsets from java.nio instead instead of hardcoding UTF8
• no need to println a empty strings
• removed unused variables
• another loop that can be early stopped
• removed unnecessary and duplicate semi-colons
• accessing static attribute using class instead of object
• added comments pointing to lists that are populated, but never used <-- this I need to review, that class was created hastily to get results for a paper, and as soon as the paper was submitted, never looked at again. So, there might be left-over code of me changing things here and there until it worked :)
• pass String directly here, instead of StringBuilder
• Exception already catches all other exceptions
• ignore config files from IntelliJ and VS Code

The following ones I want to accept, but I need time to look into them to make sure they don't break anything. So, once the previous pull request is accepted, if you could do pull requests one by one with these, I can look at them separately:

• reformatted codebase using Google Style Guide (with 4 spaces, not 2) <-- this is ok, but working at Google myself there is one think I dislike about the style we use at Google, which is the 80 character limit in code lines. As long as that constrainted was not enforced, this is fine! (hard to see in the diff online :)
• added EditorConfig file to enforce code standard <-- same with this one. Code standards are very personal things. I need some time to see if I can live with this standard
• simplified boolean expressions in if, return and switch statements
• using addAll, fill, clear, arraycopy and constructors to avoid loops
• early stopping loops

These ones I would not like to have them in the code. Thanks for the suggestions though!

• all arrays are declared Java-style instead of C-style
• removed unnecessary casting <-- I like your change to FrontEnd.java, but not sure why do we need the change in AbstractTraceGenerationTest :)
• using ArrayList of ArrayLists instead of vector of Lists <-- why do you want to change this? when you know the size in advance, vectors are always the faster solution.
• optimized imports <-- I like the idea of optimizing imports, but there seems to be some other changes that sneaked in into this commit ;)
• rearranged elements (overrides first, then by usage according to bfs) <-- some functions had been grouped together for being conceptually related, and this breaks that. So, not a big fan, sorry!
• removed method that exists only in Java 11 <-- I think you introduced this yourself in one of the commits in this pull request, since in the current version of the repo, taht "repeat" function is not there :)

[douglasrizzo]

I don't think I'll be able to create a pull request right now. I'll get our newest version of master and see if I can easily apply the existing commits to it. Otherwise, I'll have to do the work by hand, but that's no problem. Give me some time to prepare.

I think you should consider the "using addAll, fill, clear, arraycopy and constructors to avoid loops" commit, because I found many instances where one data structure was simply being filled with all elements of another data structure. I believe that using built-in methods from Java may be faster than using for loops in these cases.

[santiontanon]

Yes, the "using addAll, fill, clear, arraycopy and constructors to avoid loops" is one of the ones I want to include, but I put it in the second list, since it's one I want to look at each change, just to make sure nothing gets broken :)

But yeah, take your time. Your pull request rate is really fast, hahaha.