http://codereview.appspot.com/33069/diff/1/17 File core/src/main/java/com/google/test/metric/Reason.java (right): http://codereview.appspot.com/33069/diff/1/17#newcode1 Line 1: package com.google.test.metric; copyright comments http://codereview.appspot.com/33069/diff/1/17#newcode20 Line 20: Reason(String description, boolean implicit) { why not make this private ? http://codereview.appspot.com/33069/diff/1/5 File core/src/main/java/com/google/test/metric/method/op/turing/FieldAssignment.java (right): http://codereview.appspot.com/33069/diff/1/5#newcode34 Line 34: if (field != null) { will field assignment have a null fieldinfo passed to it ? And of course, ewwww, work in the constructor :D http://codereview.appspot.com/33069/diff/1/11 File core/src/main/java/com/google/test/metric/method/op/turing/LocalAssignment.java (right): http://codereview.appspot.com/33069/diff/1/11#newcode30 Line 30: if (variable != null) { mannnn, work in the constructor again ? :) http://codereview.appspot.com/33069/diff/1/6 File core/src/main/java/com/google/test/metric/report/issues/ClassIssues.java (right): http://codereview.appspot.com/33069/diff/1/6#newcode98 Line 98: any chance for some javadocs on this one, since its not simply adding, but doing something more ? http://codereview.appspot.com/33069/diff/1/10 File core/src/main/java/com/google/test/metric/report/issues/Issue.java (right): http://codereview.appspot.com/33069/diff/1/10#newcode47 Line 47: public Issue(int lineNumber, String elementName, float contributionToClassCost, IssueType type, IssueSubType subType) { > 100. Do u care ? a bunch of them in this file http://codereview.appspot.com/33069/diff/1/8 File core/src/main/java/com/google/test/metric/report/issues/IssuesReporter.java (right): http://codereview.appspot.com/33069/diff/1/8#newcode53 Line 53: void addIssuesInMethod(ClassIssues classIssues, MethodCost methodCost, ClassCost classCost) { this method is getting pretty long.... Its pretty much unreadable http://codereview.appspot.com/33069/diff/1/22 File core/src/main/resources/com/google/test/metric/report/costDetailBox.html (right): http://codereview.appspot.com/33069/diff/1/22#newcode34 Line 34: <#list issue.implications as implication> where can i see an example of this ? http://codereview.appspot.com/33069/diff/1/13 File core/src/test/java/com/google/test/metric/report/DrillDownReportTest.java (right): http://codereview.appspot.com/33069/diff/1/13#newcode71 Line 71: methodCost2.addCostSource(new MethodInvokationCost(81, methodCost1, NON_OVERRIDABLE_METHOD_CALL, Cost.cyclomatic(1), null)); i guess u use those checks for these ? http://codereview.appspot.com/33069/diff/1/13#newcode197 Line 197: private void assertStringEquals(String expected, String actual) { why the heck is this funky indent ? http://codereview.appspot.com/33069/diff/1/3 File core/src/test/java/com/google/test/metric/report/issues/IssuesReporterTest.java (right): http://codereview.appspot.com/33069/diff/1/3#newcode137 Line 137: assertEquals("com.google.test.metric.example.Lessons.SumOfPrimes1.primeness{com.google.test.metric.example.Lessons.Primeness}", issue.getElementName()); length http://codereview.appspot.com/33069/diff/1/3#newcode172 Line 172: assertEquals(4/5f, issue0.getContributionToClassCost()); is this that same test which evaluates to 4/5 instead of 4/6 ?
FYI http://codereview.appspot.com/33069/diff/1/17 File core/src/main/java/com/google/test/metric/Reason.java (right): http://codereview.appspot.com/33069/diff/1/17#newcode1 Line 1: package com.google.test.metric; On 2009/04/03 23:58:02, Shyam wrote: > copyright comments Done. http://codereview.appspot.com/33069/diff/1/17#newcode20 Line 20: Reason(String description, boolean implicit) { On 2009/04/03 23:58:02, Shyam wrote: > why not make this private ? Done. http://codereview.appspot.com/33069/diff/1/5 File core/src/main/java/com/google/test/metric/method/op/turing/FieldAssignment.java (right): http://codereview.appspot.com/33069/diff/1/5#newcode34 Line 34: if (field != null) { On 2009/04/03 23:58:02, Shyam wrote: > will field assignment have a null fieldinfo passed to it ? > And of course, ewwww, work in the constructor :D moved to caller http://codereview.appspot.com/33069/diff/1/11 File core/src/main/java/com/google/test/metric/method/op/turing/LocalAssignment.java (right): http://codereview.appspot.com/33069/diff/1/11#newcode30 Line 30: if (variable != null) { On 2009/04/03 23:58:02, Shyam wrote: > mannnn, work in the constructor again ? :) moved to caller http://codereview.appspot.com/33069/diff/1/6 File core/src/main/java/com/google/test/metric/report/issues/ClassIssues.java (right): http://codereview.appspot.com/33069/diff/1/6#newcode98 Line 98: On 2009/04/03 23:58:02, Shyam wrote: > any chance for some javadocs on this one, since its not simply adding, but doing > something more ? Done. http://codereview.appspot.com/33069/diff/1/10 File core/src/main/java/com/google/test/metric/report/issues/Issue.java (right): http://codereview.appspot.com/33069/diff/1/10#newcode47 Line 47: public Issue(int lineNumber, String elementName, float contributionToClassCost, IssueType type, IssueSubType subType) { On 2009/04/03 23:58:02, Shyam wrote: > > 100. Do u care ? a bunch of them in this file Done. http://codereview.appspot.com/33069/diff/1/8 File core/src/main/java/com/google/test/metric/report/issues/IssuesReporter.java (right): http://codereview.appspot.com/33069/diff/1/8#newcode53 Line 53: void addIssuesInMethod(ClassIssues classIssues, MethodCost methodCost, ClassCost classCost) { On 2009/04/03 23:58:02, Shyam wrote: > this method is getting pretty long.... Its pretty much unreadable will fix in later CL... http://codereview.appspot.com/33069/diff/1/22 File core/src/main/resources/com/google/test/metric/report/costDetailBox.html (right): http://codereview.appspot.com/33069/diff/1/22#newcode34 Line 34: <#list issue.implications as implication> On 2009/04/03 23:58:02, Shyam wrote: > where can i see an example of this ? > on my computer http://codereview.appspot.com/33069/diff/1/13 File core/src/test/java/com/google/test/metric/report/DrillDownReportTest.java (right): http://codereview.appspot.com/33069/diff/1/13#newcode197 Line 197: private void assertStringEquals(String expected, String actual) { On 2009/04/03 23:58:02, Shyam wrote: > why the heck is this funky indent ? tabs - fixed http://codereview.appspot.com/33069/diff/1/3 File core/src/test/java/com/google/test/metric/report/issues/IssuesReporterTest.java (right): http://codereview.appspot.com/33069/diff/1/3#newcode172 Line 172: assertEquals(4/5f, issue0.getContributionToClassCost()); On 2009/04/03 23:58:02, Shyam wrote: > is this that same test which evaluates to 4/5 instead of 4/6 ? Done.
fix Shyam's comment
