Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring WebUI Views, PluginInfo and various Util classes. #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactoring WebUI Views, PluginInfo and various Util classes. #14

wants to merge 1 commit into from

Conversation

MartinRogalla
Copy link

By Martin Rogalla(4173635) and Peter Evers(4013964).

PluginInfo

By creating our test suite we found that the values of the String
ConfigurationType are not type checked, this could potentially lead to
problems when a null value is put into the database.
In the refactoring phase, the fromString function for the enum
ConfigurationType was simplified. Java already has a function called
valueOf for enums, which basically performs the exact same task as the
fromString function. The only main difference was that it would return
a null value, but this value was always checked in all the callees and
an exception would be thrown if it was null.
Thus the possibility for the return type to be null was removed and the
fromString function was made responsible to throw the exception. This
reduced code duplication in the file, giving a better overview.

ProjectVersion

The class was tested and a few minor bugs were fixed.

PAServiceImpl

The error-handling in PAServiceImpl was cluttered, as it relied mainly
on Boolean evaluations. This form of error-handling was replaced by the
usage of exceptions. Not only does this give a better overview, but it
also provides the logger with more detailed error messages. Some other
refactorings such a method extraction were performed as well. The logic
of this class remained the same, there was only extraction of methods,
which does not have a direct effect on the code functionality, but
improves code readability. Compatibility with the rest of the components
in the system remains, as the install function will still return a
boolean depending on its success or failure.

FileUtils/DiskUtils

These two utility classes were re-implementing methods for which
existing frameworks can be used. This violates the Single
Responsibility Principle
, not on a class level, but rather on a
component level. Implementing these types of methods yourself increases
the likelihood of bugs, and clutters the project overview. Thus we
refactored all the classes which made use of this, to use the Apache
Commons IO library, specifically the org.apache.commons.io.FileUtils
and org.apache.commons.io.FilenameUtils classes were replacements for
FileUtils and DiskUtils. In order to fully test these changes it
would require a lot of time and which would inefficient if the affected
classes were not going to be refactored. Thus for these changes to be
made, we carefully analyzed the call hierarchy within the whole project,
and made the corresponding changes while checking the logic manually. In
total this allowed the removal of ~447 lines of code from the Alitheia
Core codebase.

PluginsView/ProjectsView

We have managed to reproduce the same functionality as the original
PluginView with only 10% of the original lines of code. The Open Closed
Principle is fixed, we can easily extend the template or write another
template for example if we want different designs.

Signed-off-by: Martin Jorn Rogalla [email protected]
Signed-off-by: Peter Evers [email protected]

PluginInfo

By creating our test suite we found that the values of the `String`
`ConfigurationType` are not type checked, this could potentially lead to
problems when a null value is put into the database.
In the refactoring phase, the `fromString` function for the enum
`ConfigurationType` was simplified. Java already has a function called
`valueOf` for enums, which basically performs the exact same task as the
`fromString` function. The only main difference was that it would return
a null value, but this value was always checked in all the callees and
an exception would be thrown if it was null.
Thus the possibility for the return type to be null was removed and the
`fromString` function was made responsible to throw the exception. This
reduced code duplication in the file, giving a better overview.

ProjectVersion

The class was tested and a few minor bugs were fixed.

PAServiceImpl

The error-handling in `PAServiceImpl` was cluttered, as it relied mainly
on Boolean evaluations. This form of error-handling was replaced by the
usage of exceptions. Not only does this give a better overview, but it
also provides the logger with more detailed error messages. Some other
refactorings such a method extraction were performed as well. The logic
of this class remained the same, there was only extraction of methods,
which does not have a direct effect on the code functionality, but
improves code readability. Compatibility with the rest of the components
in the system remains, as the install function will still return a
boolean depending on its success or failure.

FileUtils/DiskUtils

These two utility classes were re-implementing methods for which
existing frameworks can be used. This violates the *Single
Responsibility Principle*, not on a class level, but rather on a
component level. Implementing these types of methods yourself increases
the likelihood of bugs, and clutters the project overview. Thus we
refactored all the classes which made use of this, to use the Apache
Commons IO library, specifically the `org.apache.commons.io.FileUtils`
and `org.apache.commons.io.FilenameUtils` classes were replacements for
`FileUtils` and `DiskUtils`. In order to fully test these changes it
would require a lot of time and which would inefficient if the affected
classes were not going to be refactored. Thus for these changes to be
made, we carefully analyzed the call hierarchy within the whole project,
and made the corresponding changes while checking the logic manually. In
total this allowed the removal of ~447 lines of code from the Alitheia
Core codebase.

PluginsView/ProjectsView

We have managed to reproduce the same functionality as the original
PluginView with only 10% of the original lines of code. The Open Closed
Principle is fixed, we can easily extend the template or write another
template for example if we want different designs.

Signed-off-by: Martin Jorn Rogalla <[email protected]>
Signed-off-by: Peter Evers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant