HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Project Euler #18 - Max path in a triangle

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pathtriangleprojectmaxeuler

Problem

I've been learning Java for the past 6 months online and haven't had anyone review my code before. I'm worried that I may be making lots of beginner mistakes and was wondering if anyone could point out any ideas on how to make the code more OO.

The class is instantiated from the main and all relevant code is here except for validation methods which are in super. My use of the constructor concerns me, but I don't see an obvious alternative, because my main will become a beast if I put the procedural stuff for 30 classes in there. Overall, the code feels pretty procedural, and a bit spaghetti. (moreso in this example than many, the interrelated triangles don't lend themselves to being black box)

Anyways, ideas, suggestions?

```
/*
* To change this license header, choose License Headers in Project Properties.
* To change this template file, choose Tools | Templates
* and open the template in the editor.
*/

package projecteuler;

/**
*
* @author Rar Rarr
*/
public class Problem18 extends Problems implements TalkNice{

private boolean useDefaultTriangle = false;
private int triangleSize;
private int [] triangle [];
private String [] pathingTriangle [];
private int[] printingTriangle[];
private String pathingInformation = new String();

Problem18(){
explainProblem();
useDefaultTriangle();
if(!useDefaultTriangle){
inputTriangleSize();
createPathingTriangle();
calculatePath(createTriangle());
}
else{
createPathingTriangle();
calculatePath(createDefaultTriangle());
}
printTriangle(triangle);
}

@Override
public final void explainProblem() {
System.out.println("By starting at the top of the triangle below and moving to adjacent numbers on the row below, the maximum total from top to bottom is 23.");
System.out.println("");
System.out.println("");
System.out.println("

Solution

Just some points about the general structure, and a couple of smaller things that I noticed:

OOP

Constructor and general OOP


My use of the constructor concerns me, but I don't see an obvious alternative

Yes, it does seem a little odd. The constructor should only construct an object. Yours already solves the problem.

I would at least pull out explainProblem and the problem solving code (createPathingTriangle, calculatePath, printTriangle) to a method called solveProblem defined in Problems. Printing the solution could also be extracted, as it is a different activity from solving the problem.

Does verifyPositiveInt get user input? If so, that should also be pulled out of the constructor. Ideally, you would create a new interface that retrieves user input, instead of having Problems handle it.

You might also create an Output interface, so your code doesn't hardcode System.out.println. That way, you could easily switch to eg writing to a file.

It doesn't really make your main too big. It would look like this:

Problem18 problem18 = new Problem18();
    problem18.explainProblem();
    problem18.applyUserInput(new ConsoleInput());
    problem18.solveProblem();
    problem18.printSolution(new ConsoleOutput());


It has the advantage that you can easily only explain the problem or only solve the problem. And if you are worrying about a too big main method, extract that code into a function, so every problem becomes a one-liner.

Triangle

You might also consider creating a Triangle class. Right now, the responsibility for maintaining a printable triangle is mixed in with the general code. Your code might be easier to read if this functionality was extracted.

Other

createDefaultTriangle

Your for - switch seems entirely unneeded here. Just assign the values directly:

triangle[0] = new int[]{4, 62, 98, 27, 23, 9, 70, 98, 73, 93, 38, 53, 60, 4, 23};


Also, if you were using the switch, start with y = 0, that way you don't need to subtract 1 from y everywhere.

Using String constructor

Don't use the String constructor. Instead of

String str = new String("00" + String.valueOf(x));


write

String str = "00" + String.valueOf(x);


unnecessary temporary variables

You don't need to save values to a variable if you return them directly afterwards. Instead of

String str = "00" + String.valueOf(x);
return str;


write

return "00" + String.valueOf(x);


Spaces when using an Array

This:

triangle [y][x] += triangle [y - 1][x];


Looks odd to me. The standard would be to not use spaces:

triangle[y][x] += triangle[y - 1][x];


Magic Numbers

Don't hardcode special numbers. Instead, define them in a field with a proper name. For example, what is the significance of 4 - 2? The code doesn't tell me, but a properly named field might.

Define variables where they are used

For example, you are defining xAxis2 before a for loop, but it isn't used until after that loop. Move the definition to where it is actually used.

Method, Class, and Field comments

While some methods such as explainProblem don't necessarily need a comment, the entire class should have one (stating what the problem is), and most other methods should have one as well.

Fields should also have a comment. For example, what is the difference between triangle, pathingTriangle, printingTriangle, and pathingInformation? Ideally, anyone reading your code has at least a good idea what these variables will be used for without first understanding all your code.

Other comments

Inside methods, I would put more comments (but I sometimes like to write too many comments, so take it with a grain of salt). For example, why does y start at 1 instead of 0 in calculatePath? What does the y switch do in createDefaultTriangle, etc?

Also, the comments you do have start very far to the right. I would place them above the code they are commenting (otherwise, some people might have to scroll to read them).

Code Snippets

Problem18 problem18 = new Problem18();
    problem18.explainProblem();
    problem18.applyUserInput(new ConsoleInput());
    problem18.solveProblem();
    problem18.printSolution(new ConsoleOutput());
triangle[0] = new int[]{4, 62, 98, 27, 23, 9, 70, 98, 73, 93, 38, 53, 60, 4, 23};
String str = new String("00" + String.valueOf(x));
String str = "00" + String.valueOf(x);
String str = "00" + String.valueOf(x);
return str;

Context

StackExchange Code Review Q#61846, answer score: 4

Revisions (0)

No revisions yet.