PiRSquared17 / synoptic

Automatically exported from code.google.com/p/synoptic
0 stars 0 forks source link

Refactor the Java/JS ModelGraphic interface #208

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The GWTToJSUtils class manually translates Java collections of edges/nodes into 
arrays of JavaScriptObject instances. Instead, we should be accessing Java 
fields from JS directly, and/or using JS overlay types. This would eliminate a 
lot of unnecessary code that translates between Java and JS.

Relevant documentation:
http://code.google.com/p/google-web-toolkit-doc-1-4/wiki/DevGuideJavaFromJavaScr
ipt#Accessing_Java_fields_from_JavaScript
http://code.google.com/p/synoptic/issues/entryhttp://code.google.com/webtoolkit/
doc/1.6/DevGuideCodingBasics.html#DevGuideOverlayTypes

Original issue reported on code.google.com by bestchai on 31 Jan 2012 at 8:09

GoogleCodeExporter commented 9 years ago
List of step-by-step TODOs:

1. Remove graphhandler.js by shifting all of the functionality in this file 
into the ModelGraphic class.

2. Remove GWTToJSUtils class by passing a list of GWTEdge and GWTNode instances 
directly, instead of converting these into JavaScriptObject instances.

3. Merge ModelGraphic with GWTGraph by (1) shifting functionality into 
GWTGraph, and by (2) using the GWTGraph methods like getNodes() and getEdges().

4. Make GWTEdge/GWTNode instances aware of Dracula node/edge instances by 
including refs to them in the GWTEdge/GWTNode instances.

5. Change all accessors of Dracula node/edge instances and their properties to 
instead use GWTNode/GWTEdge instances.

6. Refactor the style from native JS code into the CSS style file.

7. Clean up and tighten the JavaScript code by using higher order functions, 
like map.

Original comment by bestchai on 18 Feb 2012 at 1:04

GoogleCodeExporter commented 9 years ago
graphhandler.js has been moved into ModelGraphic.  There's still some datatypes 
that are generic JavaScriptObject's, but everything seems to run fine in 
revision ef9ddc54dd81.  The only problems that seem to come up so far is that I 
think there might need to be some sort of reset for the ModelGraphic instance, 
as I seem to get null pointer exceptions when parsing the graph a second time 
and then trying to view any arbitrary paths through even a single node.

If I need to change up the implementations of the fields, I can, but otherwise, 
this section of the refactoring seems ready for code review.

Original comment by a.w.davi...@gmail.com on 25 Feb 2012 at 4:35

GoogleCodeExporter commented 9 years ago
Okay, this looks good. Make sure you fix this bug before moving on to the other 
refactoring steps. I think (3) and (4) should be next.

One important refactoring to add to to the list is to break up the 
getNodeRenderer() function. With respect to the above list of refactorings, 
this should be in position (4.5). The list of refactoring steps for this 
function is:

(4.5a) Remove the nested anonymous JS function declarations and replace these 
with named functions within ModelGraphic.

(4.5b) The contents of node/rectangle callbacks, like onmouseup and 
onmouseover, should be refactored into GWTNode methods. These callbacks should 
still be declared, but they should not do any processing themselves but merely 
invoke the GWTNode methods. 

Original comment by bestchai on 26 Feb 2012 at 8:02

GoogleCodeExporter commented 9 years ago
It's going to be a bit hairy, and I don't know how well this will really cut 
down on the data duplication, but I've managed to get some JavaScript overlay 
objects to serialize and get sent from the server over to the client.  So far 
the GWT Graph, Node, and Edge all contain a reference to a JSO named JSEdge, 
Node, and Graph respectively.  Any maniuplations to these objects can then be 
sent through one of the JSO methods directly in JavaScript (or so I would 
think).

Original comment by a.w.davi...@gmail.com on 29 Feb 2012 at 10:02

GoogleCodeExporter commented 9 years ago
So now stuff like this could (hopefully) be run on modelTab:

GWTGraph graph = serverDataStuff;

graph.create(/* necessary fields */);
graph.setEdgeColor("Green");
graph.showEdgeWeights();

And hopefully many more.  This will likely end up pulling out quite a bit of 
unnecessary code from ModelTab, thankfully.

Original comment by a.w.davi...@gmail.com on 29 Feb 2012 at 10:06

GoogleCodeExporter commented 9 years ago
I've got all functionality working (mostly) except for switching between 
showing edge weights and showing edge counts, and refining the graph.  There's 
still a bit of a bug I can't quite seem to figure out where if one switches 
back to the input panel and parses a log without refreshing the page, none of 
the view paths calls will work properly.

There's a new bug that has been introduced (I'm not sure what's causing it at 
the moment): when the model gets resized, none of the style changes affect the 
nodes, but all of the event handlers do, i.e. when clicking on a node to view 
log lines, log lines show up, but the node isn't highlighted (and everything 
else that affects node style).

Original comment by a.w.davi...@gmail.com on 2 Mar 2012 at 4:39

GoogleCodeExporter commented 9 years ago
All that's left now is changing between probabilities and counts on edges, as 
well as fixing this persistent bug when redrawing the model.

Original comment by a.w.davi...@gmail.com on 2 Mar 2012 at 5:31

GoogleCodeExporter commented 9 years ago
As of revision e3f5ede5bd6d, all of the functionality has been moved, and any 
bugs that I can think of (except for the one outlined in JSGraph's 
"highlightEdges" method (something odd going on with hash maps) have been fixed.

Please review.

Original comment by a.w.davi...@gmail.com on 7 Mar 2012 at 8:58

GoogleCodeExporter commented 9 years ago
Merged branch code into default with revision edde396cf3f3. There are a few 
minor outstanding tasks, like re-factoring CSS rules, but otherwise the new 
implementation looks great.

Original comment by bestchai on 16 Mar 2012 at 8:29