Skip to content

Cleanup: TemplateContentModelImpl needs a lot of attention #8

Description

@dhughes-xumak

TemplateContentModelImpl has several issues which will make it difficult for external contributors to work with it. Examples:

  • General organization. There are public, private, protected, and default visibility methods scattered throughout the class. There is also an enum and a field defined in seemingly random locations between methods. Perhaps these could be reordered to promote readability and clarity.
  • Methods with default visibility (package-private) are a code smell. If possible, let's get rid of these.
  • Some methods are synchronized, but the class as a whole is not thread-safe. Do these methods need to be synchronized? Could a comment be added to provide some explanation?
  • Some fields are volatile, but I don't think this provides the thread-safety that they were intended to. Reconsider these.
  • There are unused methods such as newInstance(). There is also commented out code. Remove these.
  • There are unnecessary methods which should be removed, such as response() (is only used internally by wrappedResponse(), and simply returns a field) and toJSONString() (used only by toString()). Likewise, there is a non-private inner class CharResponseWrapper which is only used once, internally. That could be inlined as an anonymous inner class.
  • Generally, I don't think wrappedResponse() belongs in this class. The response wrapping should probably be moved to the calling class: IncludeResourceHelperFunction.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions