Mantis Bug Tracker

View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0000642OLVGeneral Featurespublic2012-08-05 22:432012-08-29 20:17
ReporterFred 
Assigned ToFred 
PriorityhighSeverityblockReproducibilityN/A
StatusclosedResolutionfixed 
PlatformAllOSAllOS VersionAll
Product Version0.0.3-SNAPSHOT 
Target Version0.0.3Fixed in Version0.0.3 
Summary0000642: Full code review
DescriptionGo over all files casually and make a list of shit that's blatantly wrong
Additional InformationPrerequisite of 0000636
TagsNo tags attached.
Attached Files

- Relationships
child of 0000636closedFred Pre-0.0.3 release code clean-up using mvn site. 

-  Notes
User avatar (0001914)
Fred (administrator)
2012-08-22 22:16

MacOSAboutHandler has copy/paste tell tails by way of spaces, not tabs, not caught style checker due to embedded in temp comment.

AboutFrame Line 105 is fine 136 is not: http://stackoverflow.com/questions/5234147/why-stringbuilder-when-there-is-string [^]

SingleGraphPanel constructor sets fields to null (they already are).

Occasional // comments with //this form

GraphPositionPanel roundDecimalsOnlyToTwoSignificantFigures versus MathUtils roundToSignificantFigures Only one copy/version/process, done as text, not as number, unless I am utterly confused.

EntireGraphingPanel has a bunch of minor white space issues in private final Action defs. Suggest turning on "show whitespace" in eclipse editor so you can see it and so that it's ugly for you too ;-)

EntireGraphingPanel line 200 has three calls to get current time, which could all be different, intended? if not, change to one call into a temp final var, if so, comment to explain.

Consider this vs this from InitialLineColoring
    private static final float ONE_THIRD = 1F/3F;
    private static final float ONE_THIRD = 0.3333F;

Also, why not 1F?
    private static final float ALMOST_ONE = 0.999F;

From OpenLogViewer: commented out code, ctrl / gets you a block comment automatically, you did it manually.

From OpenLogViewer exitFullScreen method, idents should be tabs, good, post code to comments should be spaces, bad, is tabs.

Can't see anything else obvious. Didn't run site myself.
User avatar (0001915)
BenFenner (developer)
2012-08-23 14:09
edited on: 2012-08-23 14:48

Git hash: 8fe16a9ae4dcbf2f13d209f5d48c240140014f18

Includes the following work:

AboutFrame line 136 uses StringBuilder now. (Also found a use for it in SingleProperty.)

SingleGraphPanel constructor no longer sets fields to null that are already null.

All comments should now be // in this form and not //in this form.

EntireGraphingPanel is cleaned of a bunch of minor whitespace issues. Show whitespace is now on for me in Eclipse.

EntireGraphingPanel line 200 no longer has three calls to System.getCurrentTime() with added comment too.

InitialLineColoring uses 1F/3F and 2F/3F now instead of 0.3333F and 0.6666F.

InitialLineColoring has explanation now of why the use of 0.999F.

OpenLogViewer commented out code is automatic now instead of manual.

OpenLogViewer exitFullScreen method uses spacing to indent comments after code instead of tabs. enterFullScreen gets the same treatment too.





Things not addressed yet:

MacOSAboutHandler has copy/paste tell tails by way of spaces, not tabs, not caught style checker due to embedded in temp comment.


GraphPositionPanel roundDecimalsOnlyToTwoSignificantFigures versus MathUtils roundToSignificantFigures Only one copy/version/process, done as text, not as number, unless I am utterly confused.

User avatar (0001916)
BenFenner (developer)
2012-08-23 14:47

Git hash: 308a5074d19cc1839f4433f2aab17c17db484f42

Replaced spaces with tabs where appropriate in MacOSAboutHandler.





Things not addressed yet:

GraphPositionPanel roundDecimalsOnlyToTwoSignificantFigures versus MathUtils roundToSignificantFigures Only one copy/version/process, done as text, not as number, unless I am utterly confused.
User avatar (0001944)
BenFenner (developer)
2012-08-28 17:35

The last remaining issue (the significant figure code stuff) has been taken care of.

Git hash: 4bbeed7f2efe5aba9d4a0c5e88604d49247549c9


I'm going to run site:site and clean things up and then hand it back to you Fred.
User avatar (0001950)
BenFenner (developer)
2012-08-28 18:43

After site code cleanup we're good to go. Checkstyle errors rose from 260 to 261. So sue me.
Resolve/close at your satisfaction.

Git hash : 9f2b24dee79a810758f85d05c9233d2df67eef68
User avatar (0001968)
Fred (administrator)
2012-08-29 20:17

Haven't checked, but I'm calling it good anyway.


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker