2
\$\begingroup\$

I created just for practice this leapyear script.

What do you think about it? How can I make it more efficient?

package com.company;

import java.util.Scanner;

public class Main {

public static void main(String[] args) {


    Scanner input = new Scanner(System.in);
    System.out.printf("Plese enter year: ");
    int chosedYear = input.nextInt();

    leapYearMethod(chosedYear);


}


public static void leapYearMethod (int leapyear) {
    if ( leapyear % 4 == 0 ){
        System.out.println("Yeeees!");
    } else {
        int years = leapyear % 4;
        if(years == 1){
            System.out.println("3 years later there is a leapyear.");
        } else if( years == 2) {
            System.out.println("2 years later there is a leapyear.");
        } else if( years == 3) {
            System.out.println("1 years later there is a leapyear.");
        }
    }
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ As you venture into the world of nit-picking (say hello programmers ;-)) you should be aware that your algorithm of leap-year calculation is not correct. See "Algorithm" in en.wikipedia.org/wiki/Leap_year for the correct calculation rules. \$\endgroup\$
    – mtj
    Sep 3, 2018 at 10:35

2 Answers 2

1
\$\begingroup\$

Regardless of your algorithm, you can simplify it much more with a simple if or switch.

int years = leapyear%4;
switch( years ) {
  case 0: 
    println("Yeeees!");
    break;
  case 1:
    println("1 year later there is a leapyear.");
    break;
  default:
    println((4-years)+" years later there is a leapyear.");
    break;
}

But I doubt that this is a good practising exercise:

Do you have an unit test for it ? If you had to create one, how could you test the result since it is printed to the console.

How could you improve this code to make it testable and extendable. What could you do if I want to print another message ?

\$\endgroup\$
0
\$\begingroup\$

You should always close closeable resources such as Scanner. The preferred way to do this is a try-with-resources block

try (final Scanner input = new Scanner(System.in)) {
    //do stuff
}

You should prefer making variables final where possible to let readers know the assigned value won't be changing.

You can do math instead of having a big if-else if-else if block.

System.out.println((4 - years) + " years later there is a leap year");
\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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