Histogram chart is mostly whitespace

[aeagl...@gmail.com]

Reviewers: Shyam,

Description:
It's a waste of real estate. This new chart is rendered by the app,
rather than using Google charts API (as requested by a user). The Y axis
is logarithmic, so one or two classes with a high score still show
prominently. Doesn't do the green/yellow/red in the graph like the old
one did.

Please review this at http://codereview.appspot.com/32071

Affected files:
core/pom.xml
core/src/main/java/com/google/test/metric/report/GoodnessChart.java
core/src/main/java/com/google/test/metric/report/GoogleChartAPI.java
core/src/main/java/com/google/test/metric/report/GradeCategories.java
core/src/main/java/com/google/test/metric/report/Histogram.java
core/src/main/java/com/google/test/metric/report/HistogramChartUrl.java
core/src/main/java/com/google/test/metric/report/PieChartUrl.java
core/src/main/java/com/google/test/metric/report/PieGraph.java
core/src/main/java/com/google/test/metric/report/SummaryGraphReport.java
core/src/main/java/com/google/test/metric/report/TextHistogram.java
core/src/main/java/com/google/test/metric/report/TextReport.java

core/src/main/java/com/google/test/metric/report/chart/CostDistributionChart.java
core/src/main/java/com/google/test/metric/report/chart/GoodnessChart.java
core/src/main/java/com/google/test/metric/report/chart/GoogleChartAPI.java
core/src/main/java/com/google/test/metric/report/chart/Histogram.java

core/src/main/java/com/google/test/metric/report/chart/HistogramChartUrl.java

core/src/main/java/com/google/test/metric/report/chart/IntegerTickUnitSource.java
core/src/main/java/com/google/test/metric/report/chart/PieChartUrl.java
core/src/main/java/com/google/test/metric/report/chart/PieGraph.java
core/src/main/java/com/google/test/metric/report/html/HtmlReport.java
core/src/test/java/com/google/test/metric/report/GoodnessChartTest.java
core/src/test/java/com/google/test/metric/report/GoogleChartAPITest.java
core/src/test/java/com/google/test/metric/report/GradeCategoriesTest.java

core/src/test/java/com/google/test/metric/report/HistogramChartUrlTest.java
core/src/test/java/com/google/test/metric/report/HistogramTest.java
core/src/test/java/com/google/test/metric/report/PieChartUrlTest.java
core/src/test/java/com/google/test/metric/report/PieGraphTest.java

core/src/test/java/com/google/test/metric/report/chart/CostDistributionChartTest.java

core/src/test/java/com/google/test/metric/report/chart/GoodnessChartTest.java

core/src/test/java/com/google/test/metric/report/chart/GoogleChartAPITest.java

core/src/test/java/com/google/test/metric/report/chart/GradeCategoriesTest.java

core/src/test/java/com/google/test/metric/report/chart/HistogramChartUrlTest.java
core/src/test/java/com/google/test/metric/report/chart/HistogramTest.java

core/src/test/java/com/google/test/metric/report/chart/PieChartUrlTest.java
core/src/test/java/com/google/test/metric/report/chart/PieGraphTest.java

[aeagl...@gmail.com]

Thanks for the review

http://codereview.appspot.com/32071/diff/1/22
File
core/src/main/java/com/google/test/metric/report/chart/CostDistributionChart.java
(right):

http://codereview.appspot.com/32071/diff/1/22#newcode1
Line 1: package com.google.test.metric.report.chart;
On 2009/03/31 04:57:25, Shyam wrote:
> any copyright comments needed ?

Done.

http://codereview.appspot.com/32071/diff/1/3
File
core/src/main/java/com/google/test/metric/report/chart/GoodnessChart.java
(right):

http://codereview.appspot.com/32071/diff/1/3#newcode2
Line 2: * Copyright 2007 Google Inc.
On 2009/03/31 04:57:25, Shyam wrote:
> year ?

this is just a move (too bad the code review tool doesn't say so.. maybe
if I used the python uploader?)

http://codereview.appspot.com/32071/diff/1/3#newcode18
Line 18: public class GoodnessChart extends GoogleChartAPI {
On 2009/03/31 04:57:25, Shyam wrote:
> why not use jfreechart or googlechartapi for all ? or is that the
plan, but ur
> doing it in iterations ?

Yes, I think I ought to replace them all, good point. I need to get a
handle on all the graphs in the current codebase and do some more
refactors probably.

http://codereview.appspot.com/32071/diff/1/18
File
core/src/main/java/com/google/test/metric/report/chart/Histogram.java
(right):

http://codereview.appspot.com/32071/diff/1/18#newcode22
Line 22: public class Histogram {
On 2009/03/31 04:57:25, Shyam wrote:
> i'm assuming all of these are just moves into this package ?

yup, just moves

http://codereview.appspot.com/32071/diff/1/33
File
core/src/main/java/com/google/test/metric/report/chart/IntegerTickUnitSource.java
(right):

http://codereview.appspot.com/32071/diff/1/33#newcode1
Line 1: package com.google.test.metric.report.chart;
On 2009/03/31 04:57:25, Shyam wrote:
> copyrihgt

not actually using this class anymore.

http://codereview.appspot.com/32071/diff/1/6
File
core/src/main/java/com/google/test/metric/report/html/HtmlReport.java
(right):

http://codereview.appspot.com/32071/diff/1/6#newcode59
Line 59: public String getHistogram() {
On 2009/03/31 04:57:25, Shyam wrote:
> is this still a histogram ?

yes, it still is - histogram shows the distribution

http://codereview.appspot.com/32071/diff/1/21
File
core/src/test/java/com/google/test/metric/report/chart/CostDistributionChartTest.java
(right):

http://codereview.appspot.com/32071/diff/1/21#newcode1
Line 1: package com.google.test.metric.report.chart;
On 2009/03/31 04:57:25, Shyam wrote:
> copyright

Done.

http://codereview.appspot.com/32071/diff/1/21#newcode17
Line 17: protected void setUp() throws Exception {
On 2009/03/31 04:57:25, Shyam wrote:
> get rid of it, its useless

Done.

http://codereview.appspot.com/32071/diff/1/21#newcode21
Line 21: public void testCreateChart() throws Exception {
On 2009/03/31 04:57:25, Shyam wrote:
> is this test even testing anything ? delete or revert if no asseerts

it's mostly to make sure it doesn't throw exceptions, then the resulting
chart has to be manually inspected. Added asserts that the file is
created and has size > 1

http://codereview.appspot.com/32071