1
\$\begingroup\$

Question:- Why is this code so ugly ??

This is my first code, it is working fine, but I feel it doesn't looks as standard code … I am still studying java through youtube, as I have just completed two topics language fundamentals and operators and assignments in java.
I have tried to create code that calculates loan on particular date and at particular rate of interest, it takes user inputs. Please Run This Code At Your IDE.

I have created this code by just knowing two topics or say first two chapters of basic java.


package learningJava;
import java.util.Scanner;

public class Test
{

    public static void main(String[] args) 


     {
     Scanner sc = new Scanner (System.in);

    System.out.println("if Days is not calculated just type 0");
 
    System.out.print("Days: ");

    double day = sc.nextDouble();


    if (day<=0)
    {
    System.out.println("Old Year: ");

    double oldyear = sc.nextDouble();

    System.out.println("Old Month: ");

    double oldmonth = sc.nextDouble();

    System.out.println("Old Date: ");

    double olddate = sc.nextDouble();

    System.out.println("New Year: ");

    double newyear = sc.nextDouble();

    System.out.println("New Month: ");

    double newmonth = sc.nextDouble();

    System.out.println("New Date: ");

    double newdate = sc.nextDouble();



     day = ((newyear*365)+(newmonth*30)+(newdate))-(((oldyear*365)+(oldmonth*30)+(olddate)));

     System.out.println("Days = "+day);

     System.out.print("Principle: ");

     double principle = sc.nextDouble();
 
     System.out.print("Rate Of Interest: ");

     double rateofinterest = sc.nextDouble();

     double interest = (((principle*(rateofinterest/100))/30)*day);

     System.out.println("--------------------------------");

     System.out.println("Interest = Rs " +interest);

     System.out.println("                                 ");

     System.out.println("Total Amount = Rs " +(principle+interest));

     System.out.println("--------------------------------");

     System.out.println("Thankyou!!");

    }

else 
    {
    
     System.out.print("Principle: ");
     
     double principle = sc.nextDouble();

     System.out.print("Rate Of Interest: ");

     double rateofinterest = sc.nextDouble();  

     double interest = (((principle*(rateofinterest/100))/30)*day);

     System.out.println("--------------------------------");

     System.out.println("Interest = Rs " +interest);

     System.out.println("                                 ");

     System.out.println("Total Amount = Rs " +(principle+interest));

     System.out.println("--------------------------------");

     System.out.println("Thankyou!!");

     }

}

}
\$\endgroup\$
2
  • 6
    \$\begingroup\$ Try formatting your code to comply with the Java coding conventions and see how it changes your preception: oracle.com/technetwork/java/codeconvtoc-136057.html \$\endgroup\$ – TorbenPutkonen May 26 '20 at 8:58
  • 1
    \$\begingroup\$ The current question title of your question is too generic to be helpful. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$ – BCdotWEB May 27 '20 at 14:25
5
\$\begingroup\$

it is a bit hard to compare it with "standard code" if you only worked through first two chapters ;)

You should consider three principles in the following order:

  1. The code should work
  2. The code should be readable
  3. The code should be extendable

Only the 1st is proofable. But maybe not that easy as one might think. Consider edge cases and errors. It takes some experience to know them. But usually something like

  1. Boundary values - e.g. 0 for a lower boundary
  2. Wrong format of parameter e.g. 0,5 instead of 0.5 (for english local)
  3. Input is out of allowed boundary

There are a lot of ways to handle them, like to ask for a new value or to simply abort.

If your code is readable is matter of taste. I prefer short code blocks in methods that are named. Also repeated code should be avoided. That said here is my first concrete suggestion for you: There are common code blocks where you ask the user for a value (System.out.print(...)) and then read the value (sc.nextDouble()). You could combine them to a method, like

private static double askForDouble(String label, Scanner scanner){
    System.out.print(label);
    return scanner.nextDouble();
}

which you can use then instead of every occurance:

System.out.print("Days: ");
double day = sc.nextDouble();

becomes

double day = askForDouble("Days: ", sc);

I would also put every calculation in separate functions although that would not give any benefit at the moment. But later you could write unit tests.

There are more possible improvements, but you should go on with the tutorials to learn about them.

\$\endgroup\$
4
\$\begingroup\$

Welcome to CodeReview!

Style Guide

You mentioned that you think your code looks "ugly". This problem can be solved by reading a style guide (Google Style Guide).

Some important points:

  • Use proper intendation
  • Although you can use empty lines to structure your code, it is not useful to have empty lines after every line of code.
  • Use useful names (public class Test is not useful). Also you should use camelCase for your variable-names.
import java.util.Scanner;

public class LoanCalculator {

    public static void main(String[] args) {
        Scanner sc = new Scanner (System.in);
        System.out.println("if Days is not calculated just type 0");
        System.out.print("Days: ");
        double day = sc.nextDouble();

        if (day<=0) {
            System.out.println("Old Year: ");
            double oldYear = sc.nextDouble();

            System.out.println("Old Month: ");
            double oldMonth = sc.nextDouble();

            System.out.println("Old Date: ");
            double oldDate = sc.nextDouble();

            System.out.println("New Year: ");
            double newYear = sc.nextDouble();

            System.out.println("New Month: ");
            double newMonth = sc.nextDouble();

            System.out.println("New Date: ");
            double newDate = sc.nextDouble();

            day = ((newYear * 365) + (newMonth * 30)+(newDate)) - (((oldYear * 365) + (oldMonth * 30) + (oldDate)));
            System.out.println("Days = "+ day);
            System.out.print("Principle: ");
            double principle = sc.nextDouble();
            System.out.print("Rate Of Interest: ");
            double rateofinterest = sc.nextDouble();
            double interest = (((principle * (rateofinterest / 100)) / 30) * day);
            System.out.println("--------------------------------");
            System.out.println("Interest = Rs " + interest);
            System.out.println("                                 ");
            System.out.println("Total Amount = Rs " + (principle + interest));
            System.out.println("--------------------------------");
            System.out.println("Thank you!!");
        }

        else {
            System.out.print("Principle: ");
            double principle = sc.nextDouble();
            System.out.print("Rate Of Interest: ");
            double rateofinterest = sc.nextDouble();  
            double interest = (((principle * (rateofinterest / 100)) / 30) * day);
            System.out.println("--------------------------------");
            System.out.println("Interest = Rs " + interest);
            System.out.println("                                 ");
            System.out.println("Total Amount = Rs " + (principle + interest));
            System.out.println("--------------------------------");
            System.out.println("Thankyou!!");
        }
    }
}

Code Structure

To structure your code, it is highly recommended to use methods, especially for code that is used multiple times. One example:

    public static double getInterest(double rateofinterest, double principle, double day) {
        return (((principle * (rateofinterest / 100)) / 30) * day);
    }

Then you can just use double interest = getInterest(rateofinterest, principle, day); in your main-method.

\$\endgroup\$
2
\$\begingroup\$

Except the formatting, your code does not looks ugly. It just looks procedural.

Try to create classes for each roles. One that read and write to the console. One to compute the loan. And another to control the flow of your program.

As said by Martin Fowler, "Any fool can write code that a computer can understand. Good programmers write code that humans can understand."

"understand" is not only about being able to figure what your code does. But also about maintainability and evolvability.

\$\endgroup\$
3
  • \$\begingroup\$ Classes are overkill here. After the 2nd lesson of a tutorial, a few simple methods are already enough to learn. By just defining a few methods, the code can be made much more beautiful already. \$\endgroup\$ – Roland Illig May 26 '20 at 23:34
  • \$\begingroup\$ You are right. However, @Neeraj say that "it doesn't looks as standard code". I assume that "standard" means "production ready", and such code should be evolvable and testable. Creating classes for each responsibility will increase the testability of his code and introduce extension points. \$\endgroup\$ – gervais.b May 27 '20 at 6:16
  • \$\begingroup\$ I don't see how a class is "better testable" than a method, in this simple example code. At the beginning of a programmer's career, it's good to take very small steps. Adding classes in this early stage just increases the boilerplate code. \$\endgroup\$ – Roland Illig May 27 '20 at 19:25

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.