redesign the help statistics plot - #560
Conversation
| import java.util.LinkedHashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.*; |
There was a problem hiding this comment.
Please avoid the wildcard import.
| this.transactionRepository = transactionRepository; | ||
| setCommandData(new SubcommandData("stats", "Shows an general plot about help activity in this server")); | ||
| setCommandData(new SubcommandData("stats", "Shows an general plot about help activity in this server") | ||
| .addOption(OptionType.BOOLEAN, "darkmode", "generate a plot in dark mode.", false) |
There was a problem hiding this comment.
This should start with an uppercase letter. I would suggest
Generates the plot with a dark background.
| public void ImageDifferenceTest(){ | ||
| List<Pair<String, Plotter.Bar>> testData = testData(); | ||
| BufferedImage img1 = new Plotter(testData, "General helper statistics","subtitle").plot(); | ||
| BufferedImage img2 = readImage("src/test/resources/PlotterTest.png"); |
There was a problem hiding this comment.
Don't assume resources are files. Instead, load them as resources using getClass().getClassLoader().getResource("/PlotterTest.png"); or similar.
| throw new IOException("Image is null."); | ||
| } | ||
| return image; | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
Please just throw the exception instead. In a test, it's ok if an exception is thrown in an error case.
|
|
||
| public static boolean compareImage(BufferedImage img1, BufferedImage img2) { | ||
| if (img1 == null || img2 == null) { | ||
| return false; |
There was a problem hiding this comment.
Instead of just returning boolean and asserting it being true, please use proper assertions when they happen, e.g. assertNonNull(actual) and assertEquals(expected.getWidth(), actual.getWidth(), "image width does not match")
| for (Pair<Color, Double> element : bar.elements()) { | ||
| double value = element.second(); | ||
| int segmentHeight = (int) (height * (value / axisMax)); | ||
| if (segmentHeight <= 0) continue; |
There was a problem hiding this comment.
Please don't use continue to exit out of a loop. Instead, either create a method where you can return or use a segmentHeight > 0 condition where you perform the drawing.
|
|
||
| String label = formatMonth(entry.first()); | ||
| int labelWidth = graphics2D.getFontMetrics().stringWidth(label); | ||
| graphics2D.drawString(label, centerX - labelWidth / 2, startY + height + 65); |
There was a problem hiding this comment.
There are multiple occurences where you draw a String centered around an x-coordinate. Please create a method for centered texts.
|
|
||
| String label = formatMonth(entry.first()); | ||
| int labelWidth = graphics2D.getFontMetrics().stringWidth(label); | ||
| graphics2D.drawString(label, centerX - labelWidth / 2, startY + height + 65); |
There was a problem hiding this comment.
Please use a constant instead of a magic number here.
| return String.format("%.2f", value); | ||
| } | ||
|
|
||
| private String formatMonth(String month) { |
There was a problem hiding this comment.
This class shouldn't be specific to months. If you want to display month names differently, please adapt the callers to pass the month names in the format that works well for them.
| if (normalized <= 1) { | ||
| nice = 1; | ||
| } else if (normalized <= 2) { | ||
| nice = 2; |
There was a problem hiding this comment.
If you want to, you can change the 2 to 2.5 here to have "nice" steps of 25.
| }; | ||
|
|
||
| @Test | ||
| public void ImageDifferenceTest(){ |
There was a problem hiding this comment.
You might want to add a second test for dark mode but that's up to you.
2 participants
redesign the help statistics plot.