[ https://issues.apache.org/jira/browse/OFBIZ-6783?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Taher Alkhateeb updated OFBIZ-6783: ----------------------------------- Attachment: OFBIZ-6783.patch Hello folks, Another patch for refactoring the start component that does the following: - convert the startup loaders from ArrayList to List. Given that only two loaders existed in the system since the beginning of OFBiz means that it is really unnecessary to use the loaders.trimToSize(). It's simply too much specificity for no apparent performance value. this leads to changing many method signatures from ArrayList to list across different classes - More simplification of Config.java by simplifying timezone setting logic - remove the throws StartupException from main. This thing right here confused me a lot and lead to a regression from my refactoring in OFBIZ-7167 because some exceptions bubble up all the way to main but then the system does not terminate because other threads are running. It took me and Jacques a long while to dig it out. - Introduce a function called fullyTerminateSystem(StartupException e). This function replaces the exception bubble issue that is described above. Consequently, the methods init, status and shutdown no longer throw an exception but instead terminate the system (which is the right thing to do). - finetune adaptStartupCommandsToLoaderArgs(...) and other places in StartupControlPanel - separate the Classpath creation from NativeLibClassLoader creation. This makes it cleaner and easier to refactor in the future - break the StartupControlPanel.start(...) method into smaller more readable methods. It is easier now to understand the startup sequence for ofbiz in english words (the method names). It also allows for cleaner refactoring in the future. - add JavaDoc to a few methods in StartupControlPanel Unfortunately, the current version of trunk is failing one of the tests but not related to this patch. So you need to disable the failing test and you will observe that the system works fine. It also works fine if any test fails and does not repeat the regression of OFBIZ-7167. The failing test is testAcctgTransOnEditPoInvoice defined in AutoAcctgTransTestsPurchase.xml I feel comfortable about this patch but as usual I wait for your feedback. I think the startup logic is starting to look clear and nice. > Refactor the start component > ---------------------------- > > Key: OFBIZ-6783 > URL: https://issues.apache.org/jira/browse/OFBIZ-6783 > Project: OFBiz > Issue Type: Improvement > Components: framework > Affects Versions: Upcoming Branch > Reporter: Taher Alkhateeb > Assignee: Taher Alkhateeb > Labels: framework, main, refactoring, start > Attachments: OFBIZ-6783-hiddenfiles.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, OFBIZ-6783.patch, StartCommandUtil.java, error.log, ofbiz.log > > > Looking at the main method and design of Start.java and the start component overall looks ugly. The things I would like to fix so far are: > - the files are too long > - some variables are not even needed (loaderArgs?) > - the level of abstraction is wrong > - main throws an exception! > - the arguments processing logic is terrible, need to move it to commons-cli > It's just so messy and ugly to look at. So for me refactoring starts at Start! Given that this is an important component, I will provide a patch to be reviewed by the community before committing just to be on the safe side. -- This message was sent by Atlassian JIRA (v6.3.4#6332) |
Free forum by Nabble | Edit this page |