Refactoring suggestions

Owned by kurie
Last updated: Feb 16, 2012Version comment
3 min read
  • 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?