/
Refactoring suggestions

Refactoring suggestions

Feb 16, 2012

  • Move the tree layout data into hibernate

  • All JPA Entities

    • Can I get rid of / make private the bean setter methods and still persist and serialize entities? It adds unnecessary code and opens up fields that clients don't need to set.

    • Remove accessors for JPA id fields, which the client should never really touch.

  • View is too big.

    • Remove setStyleMap(). Unused and ugly.

    • Remove convenience methods (getTree(), getLayout()).

    • Make render() private or protected.

    • dispatch() just wraps eventBus.fireEventFromSource with a null check.

    • Subclasses must provide a camera. Add it to the constructor and remove setCamera();

    • Check if all of the zoom methods are really necessary.

    • Pull the general event-related stuff up into a superclass?

  • DetailView is too big

    • move the handleMouseX(Hit, int, int) and this.addMouseXHandler handlers into an InteractionMode

    • Simplify DetailView mouse handler changes.

      • just return the list of registrations from addMouseHandler and addKeyboardHandler, combine them and keep one list of registrations to unregister when changing interaction mode

    • Refactor the event broadcast and dispatch stuff into another class

    • All of the highlight methods seem unnecessary. Just have the caller set the highlights on the RenderPreferences.

  • OverviewView

    • Add InteractionModes to handle mouse and keyboard?

  • ViewCircular class seems unnecessary. It's just a few default settings on a DetailView. Make a factory method somewhere.

  • CanvasGraphics.clearDrawnTextExtents() shouldn't be necessary

  • Revisit why I did all of the NodeSelectionHandlers in views that just refire events with a different source. If not removing altogether, at least move this to a separate class.

  • SelectionAreaChangeEvent and its handler don't appear to be used anymore

  • Combine all of the XClickEvents and XClickHandlers into two classes with maybe an enum for the element being clicked. (see TreeItemEvent class hierarchy)

  • Change the client services to take a String tree id or root id, instead of a byte[], and then get rid of client Hex class.

  • Phyloviewer.java

    • .onModuleLoad() is a monster. refactor this.

    • style fetching and updating is messy.  refactor.

  • TreeWidget

    • Constructor shouldn't need to take a SearchServiceAsyncImpl and TreeWidget probably shouldn't need a highlighter field anyway. Handle highlighting on the event bus and leave TreeWidget a little cleaner.

    • Replace render() calls with RenderEvents? Remove render() from TreeWidget?

    • Look for a way to get exportImageURL out of it, to avoid calling TreeWidget.exportImageURL() -> ViewCladogram.exportImageURL() -> DetailView.exportImageURL()

    • remove getView(). Did most of the minor refactorings already, but need a test to make sure that SVG works before changing that call

  • Are interfaces like HasNodeSelectionHandlers necessary?

    • Just getEventBus() and addHandlerToSource()

  • RemoteTree

    • move RemoteNode.reindex() to RemoteTree

    • remove RemoteTree(String name) constructor

  • Remove serialization version IDs, add @Supress annotation.

    • Revisit why I'm using Serializable instead of isSerializable. Think it was just to make some things not depend on GWT. Allows us to pull out persistent model classes into a project that doesn't depend on gwt.

  • Constructors, in general: Remove the extra-sugary ones that just set fields. Just use the no-arg constructor and then setters. But keep copy constructors.

  • NodeTopology

    • Seems like getRootNode() is not being used. Don't remember why I needed this.

    • NodeTopology should have a reference to the node it's describing but, if I remember right, this caused persistence or serialization problems

  • SearchServiceAsyncImpl

    • Use eventbus instead of the observer pattern

  • SearchHighlighter

    • redo with better use of eventbus

    • Don't hold references to View and SearchService and ITree, and don't use their setXHandler methods, just add listeners to their event buses directly for search . (and send RenderEvents to the view)

    • At the very least, get the tree from the view's document instead of setting it. that's just plain dumb.

  • RenderPreferences

    • Shouldn't the views, etc, listen for changes on the RenderPreferences? or renderpreferences fire a render event?

    • Does everyone really need a reference to the RenderPreferences? what about just handling those changes with events?

  • Move LayoutResponse to its own file, or at least into ILayoutData

  • org.iplantc.phyloviewer.viewer.server.RenderTree just scales an image. browsers can already do this. Can we get GWT to do the scaling in the way that we need it to, consistently across browsers?

  • deprecate or remove StyleProxy?

  • Get the JPA out of SearchServiceImpl. Add a method to ITreeData to find nodes by label

  • TreeIntersectService returns a json string instead of just using the RPC to return an object... Lame.

    • Add a method to return a Hit or something.

    • remove all of org.iplantc.phyloviewer.client.math, plus JsHit.java and JsHitResult.java, any other classes used to parse the json response

  • IntersectTreeServiceImpl: shouldn't the ILayoutData access interface do the databasey stuff for this service?

  • Break the org.iplantc.phyloviewer.shared.model.metadata package out into a separate project?