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.");
 }
 }
 }
}
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Sep 1, 2018 at 15:14
\$\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\$ Commented 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 ?

answered Sep 4, 2018 at 12:40
\$\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");
answered Sep 1, 2018 at 15:43
\$\endgroup\$
1

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.