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

Seah Eu Jin, Assignment 1. Handle error for null command in PoolBasedSequentialScheduledExecutorService.java & Refactor code readability and maintainability #2

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Totroeujin
Copy link
Owner

### Assignment Part 1, WIF3005 SME, Seah Eu Jin, S2130467
Reengineering tasks chosen are Code Refactoring and Improve Error Handling.
File chosen to be worked at is openhab-core/bundles/org.openhab.core/src/main/java/org/openhab/core/common/PoolBasedSequentialScheduledExecutorService.java

Check on head 1111444 with commit message "[Added changes on code for refactoring]", the first commit did not receive the code changes

Improve Error Handling tasks:

  • Added a validateCommand() method. This method check if the argument/command passed into this method is null, if null, throw an IllegalArgument Error.
  • Insert validateCommand() method into core scheduling method such as [schedule() included overrided method, scheduleAtFixedRate(), scheduleWithFixedDelay(), execute()]

I create validateCommand() method here because validate a command/callable before passing into concurrent/parallel service is important as it may cause a catastrophic failure in an application when the service is needed and being called.

Code Refactoring

  • method shutdownNow() has been revised and extracted the underlying logic into a new method called cleanupScheduledTasks().

The shutdownNow() method's code is now cleaner and more readability with higher maintainability as the method are commented in details and operation part is being represented by a new method. The core logic for this method is now being handle by cleanupScheduledTasks() while the shutdownNow() handles the flow for the operation. Thus, achieved code clarity and readability, and as the core logic is extracted into a single method (originally cramped inside shutdownNow()), it improved the maintainability for such operation.

  • Slight Refactor is done to method such as [schedule(), scheduleAtFixedRate(), scheduleWithFixedDelay()] to improve readability.

The code for these methods had indentation not following good coding practice and inconsistent. Indentation and brackets were corrected, then the common spacing between lines are being adjust. Thus, readability for the code and method are being improved.

  • shutdown() method are being simplify and modified to prevent more parameter passing between needed classes and object.

Originally using a method that needed return value, but did not make use of the returned value, leading to waste of computation cost in terms of time for processing the return value and passing boolean value. After being modified, the method now make use of forEach loop, clearly stated operation on each object. Thus improved readability and indirectly done code optimization (although it is minimal).

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.

2 participants