Reduce code redundancy in TRView

Issue #361 new
Robert Leach created an issue

USE CASE: WHAT DO YOU WANT TO DO?

Use the same method for drawing trees on the screen as is used for drawing trees in an exported image to reduce redundancy.

STEPS TO REPRODUCE AN ISSUE (OR TRIGGER A NEW FEATURE)

  1. Peruse the code in TRView.java and note redundancies.

CURRENT BEHAVIOR

Multiple instances of code that is very similar.

EXPECTED BEHAVIOR

1 instance of code.

DEVELOPERS ONLY SECTION

SUGGESTED CHANGE (Pseudocode optional)

Rewrite the tree drawing code to account for all use cases (file export & screen drawing). Start with the export code as a base and add code related to cursor hover position. Simplify the LinearTransformation and the Rectangle object usage. Either eliminate them and make the code self-contained in TRView or make their code more logical (such that for example, the variables supplied to the LinearTransformation constructor aren't named using x & y, which implies an object applies to both dimensions, and such that an indent can be supplied for where to start drawing, other than the 0 point. Rectangles also, could be eliminated because we never draw anything less than the full height of the tree. They are also used only to supply pixel coordinates of the region you want to draw, which from an export context, is an unnecessary hoop to jump through and there is no way to specify the actual "pixel" region where the drawing boundaries are without it possibly changing the data index boundaries.

FILES AFFECTED (where the changes will be implemented) - developers only

TRView.java LinearTransformation.java and class that uses draw(), LinearTransformation, or Rectangle (in the context of specifying a region to draw)

LEVEL OF EFFORT - developers only

major

COMMENTS

I had looked into merging the various draw and export methods. To do so, I had to figure out how the LinearTransformation and Rectangle classes were used to translate index and correlation values into pixel coordinates. I figured it out and I decided that doing that is too clunky. The main reason is that it's designed to be used in coordination with a rectangle object that is created using offscreen pixel coordinates. That makes sense when you're getting those coordinates from a scrollbar. And it determines the min/max data indexes from that. However, I have the data min/max indexes, and I have the pixel/point coordinates just outside those coordinates. But in order to make the existing system work for what I need to do, I have to translate my data indexes into pixels so NodeDrawer can determine data indexes for me and there's no way for me to set the actual pixel bounds of the image (because the existing code always draws the whole tree).

I also determined that using a rectangle object to define the region of the tree to draw (in terms of offscreen pixel coordinates) is outdated, since we always draw the entire height of the tree. I was baffled by the uselessness of specifying 0 and the height of the tree every time, so I fired up java treeview 2 to take a look at why it was done that way and I found out why: when you click a node in the tree in jtv2, it draws the zoomed subtree to fill the height of the panel it's being displayed in.

All in all, what I would really need to do is re-write NodeDrawer and strip out the currently useless code and provide methods appropriate for the new set of use-cases. In fact, I think that using my export methods as a basis for the next version might be much simpler. Yet I think right now, there are other issues more important. So I'm going to create a BB issue for this. I'll look and see if I can do some minimal code reduction.

Comments (2)

  1. Robert Leach reporter
    • removed milestone

    Removing milestone: Coding Convention Adherence (automated comment)

  2. Log in to comment