3
\$\begingroup\$

I am in my first high school programming course; the assignment is:

Write a program that asks the user to input three integers and outputs a message indicating their sum and whether it is positive, negative, or zero. You should create 2 classes to run this program. One class contains all of the procedures for your program and the other has all of the procedure calls for your program.

I have done exactly what the assignment has asked for me to do; I was wondering if there was anything that I should change to make my code more simple and or more elegant or point out anything that might lead up to errors, since I'm in my first programming course. I really want to start programming with simple elegant code and not messy code, because I feel like it starts good habits for me later on.


import java.util.Scanner;

class ConorsList {
    int total;


    public void add(int number) {
        this.total = this.total + number ;       
    }

    public void status(){
        if(this.total<0) {
                System.out.println("The sum is negative.");
        } else if (this.total>0) {
                System.out.println("The sum is positive");
        } else {
               System.out.println("The sum is equal to zero");
        }
    } 

    public void status(int total){
        if(total<0) {
                System.out.println("The number is negative.");
        } else if (total>0) {
                System.out.println("The number is positive.");
        } else {
               System.out.println("The number is equal to zero.");
        }
    } 
}

public class Class1_2 {

   public static void main(String args[]) {
       ConorsList list = new ConorsList();
       Scanner fromKeyboard = new Scanner(System.in);

       int amount = 3;

       for (int n=0; n<amount; n++) {
           int x =fromKeyboard.nextInt();
            list.add(x);
            list.status();
            list.status(x);
       }

   }

}
```
\$\endgroup\$
  • 2
    \$\begingroup\$ This is actually a rather challenging exercise if you need to take into account the possibility of integer overflow during addition. You're probably not expected to do so, though, if this is a first assignment in a high school course. \$\endgroup\$ – 200_success May 1 at 13:57
  • \$\begingroup\$ can you explain a little more indepth? \$\endgroup\$ – Conor Oepkes May 1 at 14:12
  • \$\begingroup\$ An int can only go as large as 2147683487. If you add 2147383487 to 2147683487, the sum should be positive mathematically, but will be reported as -2 in Java because the result would be too large to be represented as an int. \$\endgroup\$ – 200_success May 1 at 14:22
  • \$\begingroup\$ okay cool, yeah like you said "you're probably not expected to do so" i think the assignment is just to do basic classes \$\endgroup\$ – Conor Oepkes May 1 at 14:23
0
\$\begingroup\$

Binary operators have spaces before and after them

In your mains for loop, you have:

int n=0;

It is more customary to write int n = 0;

Superfluous new lines

Given

public class ... {

   public static void main(String args[]) {
       ConorsList list = new ConorsList();
       Scanner fromKeyboard = new Scanner(System.in);

       int amount = 3;

       for (int n=0; n<amount; n++) {
           int x =fromKeyboard.nextInt();
            list.add(x);
            list.status();
            list.status(x);
       }
       // Why new line here?
   }
   // And here?
}

Superfluous space in the for loop

In the main's for loop, you have

   for (int n=0; n<amount; n++) {
       int x =fromKeyboard.nextInt();
        list.add(x);
        list.status();
        list.status(x);
   }

Should be:

    for (int n = 0; n < amount; n++) {
        int x = fromKeyboard.nextInt();
        list.add(x);
        list.status();
        list.status(x);
    }

Bad name

I suggest you rename fromKeyboard to scanner here:

Scanner scanner = new Scanner(System.in);

Superfluous variable

int amount = 3;

Having this definition does not buy you much maintainability. I see two options here:

  1. Remove it and have n < 3 in the for loop,
  2. Declare it final.

Bad class name

Class1_2 is a bad name for a class. Since it does nothing but runs you main logic, I suggest you rename it to Main, Demo, or similar.

Advice 1

When you reach a particular level of proficiency in Java, it is advisory to start using packages.

\$\endgroup\$
  • \$\begingroup\$ So i have a lot of unnecessary spaces between lines, and i can make my code less lines by getting rid of those. if i dont put spaces for the binary ops will that ever lead to an error or is it just it looks better? \$\endgroup\$ – Conor Oepkes May 1 at 13:18
  • \$\begingroup\$ No, it does not lead to errors, but it may lead to errors while reading the code. For example: for(int i=0,j=-1;i>j&&ji+10;i+=2,j-=3) \$\endgroup\$ – coderodde May 1 at 13:23
  • \$\begingroup\$ i see now, without the spaces its really hard to read without the spaces, thanks \$\endgroup\$ – Conor Oepkes May 1 at 13:24
  • \$\begingroup\$ @ConorOepkes No problem. Just remember that readability/maintainability is no less important than correctness. \$\endgroup\$ – coderodde May 1 at 13:26
  • 1
    \$\begingroup\$ Java is a strongly typed language, so there is no need to repeat the type of the variable in it's name. Especially if the type is the only thing in the name. The name "fromKeyboard" was in that sense more descriptive than "scanner". However, as System.in doesn't always come from keyboard, your name can be a bit deceptive. Names should describe what the variables do. E.g. "amount" should be "numOfEntriesToRead". \$\endgroup\$ – TorbenPutkonen May 2 at 7:53
1
\$\begingroup\$

Use shorthand for addition, multiplication, division etc.

E.G instead of this.total = this.total + number;

do: this.total += number;

You have a lot of code repetition with these two methods. Anytime you copy & paste, you should be asking yourself if you can make the code shorter.

With your overloaded method, it's easy. Just change your no-arg constructor to call the other:

public void status(){
    status(this.total);
} 

You may also want to change the name to printStatus. You can further reduce the code, since all of the if statements print a lot of the same characters

E.G:

public void status(total){
    String status = "The sum is ";

    if(total<0) {
            status += "negative.";
    } else if (total>0) {
            status += "positive";
    } else {
           status += "equal to zero.";
    }

    System.out.println(status);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.