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

String puzzle - operations with numbers inside of strings

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

Problem

Puzzle description: input is dimensions (length, width, heigth) of various presents.

Example:

2x3x1  
5x9x10  
1x40x2


Part 1: determine the area of paper needed to wrap the gifts, wich is equal to 2(lw + wh + lh) + the smallest between lw, wh and l*h for each gift.

Part 2: determine the length of ribbon needed to wrap the gift, wich is equal to lwh + smallest perimeter for each gift.

I learned about aoc recently and decided to start from 2015. Below is my code from the day 2 (problem link). How can I improve it? Is it too long?

```
import java.io.*;
import java.util.*;

public class adventofcode2 {
public static void main(String[] args){
System.out.println("input: ");
Scanner input = new Scanner(System.in);
String line;
String text = "";
while (!(line = input.nextLine()).isEmpty()){
text += line + "\n";
}
calcTotal(text);
System.out.println("Paper area used - " + TotalPaper);
System.out.println("Ribbon length used - " + TotalRibbon);
}
/**
* 1 - Calculates the area of paper necessary to make the presents wrapping
* 2 - Calculates the length of ribbon necessary to the wrapping
* @param presentsText list of strings in the style numberxnumberxnumber
*/
private static void calcTotal(String presentsText){
String[] presents = presentsText.split("\n");
for (String present : presents){
calcEachPresent(present.split("x"));
}
}
/**
* Iteration inside the calcTotal method
* @param presentText array of each present string written in
* the style numberxnumberxnumber
*/
private static void calcEachPresent(String[] presentText){
//Definition of the three dimensions of the present.
int l = Integer.parseInt(presentText[0]), w = Integer.parseInt(presentText[1]), h = Integer.parseInt(presentText[2]);

//Definition of the area of the sides i

Solution

The structure of your code looks quite good so far but you could improve readability:

Naming Conventions

As with almost every other programming language java has a schema of how to name identifiers that's common consensus You can read the details here:
http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-135099.html

The main line is: classes start with an uppercase letter, method and variable names start with lower case letter. And if the name consists of multiple words use CamelCase.

variable declarations

You declared 2 variables ab the end of your class file. The common consensus in Java is to write class (or object) scope variables at the top of the class file.

less comments ##

there should be 2 types of comments in your code:

-
Description of the contract of an interface method

this type of comment should guide the implementer of that method and tell what the caller of the method expects, especially what the properties of the parameters and the result are like.

-
Explanations why the code is like it is

this should tell the reader why you chose a certain approach.

There should not be any other comment in your code.

Whenever you feel the need to write a comment try to find better identifier names first.

Especially comments like this are not needed instead they clutter the code:

/**Get smallest perimeter to use in ribbon length calculation
     * 
     * @param l length of present
     * @param w width of present
     * @param h heigth of present
     * @return smallest perimeter
     */
    private static int smallestPerimeter(int l,int w,int h){


Instead the code could be like this:

private static int getSmallestPerimeterToUseInRibbonLengthCalculation(int length, int width, int heigth){


You may argue that the name is to long and you're right: the method should only tell what it does and not restrict the possible callers, so it should better be:

private static int getSmallestPerimeter(int length, int width, int heigth){


And get is not correct since this method is not a getter in the sense that the content of an object (or class) variable is returned, it calculates the returned value and the name should reflect that:

private static int calculateSmallestPerimeter(int length, int width, int heigth){


You use comments to structure a method:

//Definition of the three dimensions of the present.
        int l = Integer.parseInt(presentText[0]), w = Integer.parseInt(presentText[1]), h = Integer.parseInt(presentText[2]);

  //Definition of the area of the sides in the present.


In such cases you should better extract that code between the comments into separate methods or objects with names derived from the comments:

PresentDimensions.java

class PresentDimensions{
    final int length, height, width;
    PresentDimensions(int length, int width, int height){
       this.length  =  length;
       this.width  =  width;
       this.height  =  height;
    }
    public int[] calculateSurfaceAreas(){
       return new int[]{length*width,width*height,height*length}
    }


in calcEachPresent

PresentDimensions presentDimensions = new PresentDimensions(presentText[0], presentText[1], presentText[2]);
 int[] d = presentDimensions.calculateSurfaceAreas();

Code Snippets

/**Get smallest perimeter to use in ribbon length calculation
     * 
     * @param l length of present
     * @param w width of present
     * @param h heigth of present
     * @return smallest perimeter
     */
    private static int smallestPerimeter(int l,int w,int h){
private static int getSmallestPerimeterToUseInRibbonLengthCalculation(int length, int width, int heigth){
private static int getSmallestPerimeter(int length, int width, int heigth){
private static int calculateSmallestPerimeter(int length, int width, int heigth){
//Definition of the three dimensions of the present.
        int l = Integer.parseInt(presentText[0]), w = Integer.parseInt(presentText[1]), h = Integer.parseInt(presentText[2]);

  //Definition of the area of the sides in the present.

Context

StackExchange Code Review Q#151298, answer score: 3

Revisions (0)

No revisions yet.