quiz icon indicating copy to clipboard operation
quiz copied to clipboard

Pull request quiz zatziky

Open zatziky opened this issue 8 years ago • 0 comments

The class breaks several principles and can be improved in many ways. Since the class might be in production already I made only safe cosmetic changes that don't modify the behaviour. It could cause bugs otherwise.

In the following list I propose further improvements:

  1. The class breaks Single Responsibility Principle as it also handles files. File handling should be removed completely together with the methods getFile(), setFile() and saveContent() and IOExceptions.
  2. Add toString() method.
  3. Make the class immutable by sending contents (not files) directly to the public methods. (This also satisfies Josh Bloch's Design for inheritance or else prohibit it.) Then the class would be truly thread-safe and we could remove synchronized key words.
  4. As it seems like a library class the documentation should be improved.
  5. The methods getContent() and getContentWithoutUnicode() are almost identical. They break DRY principle. Their bodies can be extracted to a private function with a predicate/closure/anonymous class as an argument. The predicate would carry a condition telling how to filter out the content. I didn't refactor it in the pull request as there was no unit test I could verify functionality against it.

zatziky avatar Apr 19 '16 12:04 zatziky