GSUnitedLabs / NTAPlatform

The repo upon which the Northwind Traders Basic Edition is going to be built.
GNU General Public License v3.0
0 stars 0 forks source link

Major Refactoring and Some Implementation Created #2

Closed SeanCarrick closed 2 years ago

SeanCarrick commented 2 years ago

Note: I am starting with this note because of the number of files affected by this PR. No mind needs to be paid the individual files in this PR. The important part of the approval or rejection is, "Does it work?". This note is mostly here for @jkovalsky, because I know how he immensely despises PRs with tons of files in them. However, I promise that this will be the last one. End of Note

I have created three new modules: GS.United.Labs.API, NTA.Logging, and NTA.Properties; in an effort to shrink the size of the NTA.Foundation module. The Foundation module had been growing because I kept trying to create other modules and would end up with cyclic dependencies, so I would take the contents of the various modules and add their packages into NTA.Foundation to get rid of the cyclic dependencies. However, with NTA.Foundation growing as big as it was, I would have ended up with everything in it, which would have completely defeated the purpose of the move to the modular architecture.

So, to be able to move such things as the Logger and Properties back to their own modules, I created three new interfaces in the GS.United.Labs.API module: GSLogRecord, GSLogger, and GSProperties. I then refactored the LogRecord, Logger, and Properties to implement these new interfaces, respectively. With this change in place, we no longer directly instantiate the LogRecord or Logger classes. Instead, we use the ServiceLoader mechanism to get an instance of these two objects. Due to making this modification, the getInstance() had to go away because the ServiceLoader calls a default no-args constructor and does not provide a way of calling a getInstance method.

A problem with this change arose due to how I had the Logger naming itself. Both of the getInstance methods took the Application object. One of those methods took a String of the class name, and the other took the class itself. Either way, the constructor built the log file name as:

String logFileName = app.getContext().getResourceMap().getString("Application.name") + " - " + className + ".log";

Now, when the logFileName is built, its name ends up being "NTA-Basic - null.log" for every class that uses a Logger. This kind of defeats the purpose of the naming convention. To fix this issue, the new way to load the instance variable is:

logger = ServiceLoader<GSLogger>.load(GSLogger.class).iterator().next();
logger.setClassName(getClass().getCanonicalName());
logger.updateLogName();

Now, the Logger is initialized with the first (and, in NTA, only) Logger class that the ServiceLoader finds. And, there is no longer a cyclic dependency between NTA.Foundation and NTA.Logging. Furthermore, all of the logging functionality has been split off into its own module, which is made up of a whopping three classes. That is a proper module!

Then, taking it a step further, I did the same with the Properties class...made an interface that Properties now implements, and moved it to its own module, NTA.Properties. The only difference is that the Properties class can be directly initialized because, though it relies on the Application class of the AppFramework, it does not rely upon the NTApp class. This means that the NTA.Foundation module can require NTA.Properties without worry, unlike with the Logger class.

And, going even a step further, I refactored the com.gs.nta.api package out of the NTA.Foundation module into the GS.United.Labs.API module. NTA.Foundation is definitely having some much-needed weight loss. Do not get me wrong, it is still rather large...

After I had all of this completed, I refactored the com.gs.nta.utils package into its own module, NTA.Utils. I have also created a module called NTA.Bug.Reporter that I have barely started working on.

Even with all of this work done, I have gotten the com.gs.nta.desktop.panles.ProxyOptionsPanel almost fully functional for loading, editing, and saving proxy settings, default web browser, and anonymous usage statistics checkbox settings to the application properties and other config files. I said "almost fully functional" because I am having an issue getting the fields on the Web Browser Manager to act right when the web browser selection in the list changes. However, I do have adding and removing those browsers working fine.

So, that just about covers everything that is in this PR. Now you can see why there are so many affected files. After this one, large, PR, I promise, @jkovalsky, I will do my best to keep them reasonable. Please forgive me this once. :grin:

-SC

knathan54 commented 2 years ago

Okay, after some scrambling to figure out how to get a remote branch, I got it in and did "Clean and build" that was successful, and then clicked "Run" and I got the application main screen, I am assuming. (See attached pic.)

The "About" box looks good, but I get some exceptions when I click on "Tools -> Options." I will attach a file of the Output window in Netbeans named "refactoring-away-from-cyclic-dependencies.txt".

refactoring-away-from-cyclic-dependencies.txt

refactoring-away-from-cyclic-dependencies

SeanCarrick commented 2 years ago

With no new information and the inability to recreate the exceptions on my end, I am going to go ahead and merge this request to start working on the next item on my TODO list: the NTA.Bug.Reporter module. This next task will assist in tracking down what could possibly be causing the exceptions that @knathan54 is receiving when he tries to run the project...

-SC