Age Distance Calculator

Here is my final build of the Age Distance Calculator - Any feedback on the code would be great - This is my 2nd day of java programming and thought this was a good little project.

package dev.swift.atc;

import java.util.Scanner;

import java.util.concurrent.TimeUnit;

public class AgeCalc {

private int personAge;
private int age;
private int d_age;
private String name;
static Scanner s = new Scanner(System.in);

public static void main(String [] args) throws InterruptedException
{
AgeCalc ac = new AgeCalc();
ac.getInputs();
boolean result = ac.calculateAge();
if (result == true) {
ac.showResults();
s.close();
}
else {
ac.errorResults();
TimeUnit.SECONDS.sleep(3);
System.out.println("Restarting Program");
TimeUnit.SECONDS.sleep(2);
ac.ageCalculateTwo();
}
}
//The constructor
public AgeCalc() throws InterruptedException {
System.out.println("Welcome To Java Coded Age Differnce Calculator");
TimeUnit.MILLISECONDS.sleep(100);
}

//Handle inputs
public void getInputs() throws InterruptedException {
System.out.println("Input Your Name Before We Start");
this.name = s.nextLine();
TimeUnit.MILLISECONDS.sleep(100);
this.age = s.nextInt();
TimeUnit.MILLISECONDS.sleep(100);
System.out.println("Your Age Is " + this.age + " Years Old.");
}

//Make the calculation
public boolean calculateAge() throws InterruptedException {
TimeUnit.MILLISECONDS.sleep(500);
System.out.println("Now Input The Desired Age To Work Out The Distance To");
this.d_age = s.nextInt();
if (this.d_age < this.age){
System.out.println("You Cannot Set The Target To Lower Then Your Current Age");
return false;
}
else {
this.personAge = this.d_age - this.age;
return true;
}
}

//Show the results
public void showResults() throws InterruptedException {
System.out.println("Working - Coded By Swift, Hope You Will Use This Program Again " + this.name);
TimeUnit.SECONDS.sleep(3);
System.out.println("You Will Be " + this.d_age + " In " + this.personAge +" Years");
TimeUnit.SECONDS.sleep(3);
System.out.println("Hope You Enjoyed The Program " + this.name);
System.exit(0);
System.out.println("Line Never Reached - Test");
}
public void errorResults() throws InterruptedException {
System.out.println("You Have Entered An Error " + this.name);
}
public void ageCalculateTwo() throws InterruptedException {
TimeUnit.MILLISECONDS.sleep(500);
System.out.println("Now Input The Desired Age To Work Out The Distance To - Remember Not To Be Lower Than " + this.age);
this.d_age = s.nextInt();
if (this.d_age < this.age){
System.out.println("You Have Already Been Told Not To Do This");
TimeUnit.SECONDS.sleep(3);
System.out.println("Program Shutting Down " + this.name + "Due To Multiple Errors You Have Caused ");
TimeUnit.SECONDS.sleep(2);
System.out.println("Closed - Coded By Swift, Hope You Will Use This Program Again " + this.name);
TimeUnit.SECONDS.sleep(2);
System.exit(0);
System.out.println("Line Never Reached - Test");
}
else {
this.personAge = this.d_age - this.age;
System.out.println("Working - Coded By Swift, Hope You Will Use This Program Again " + this.name);
TimeUnit.SECONDS.sleep(3);
System.out.println("You Will Be " + this.d_age + " In " + this.personAge +" Years");
TimeUnit.SECONDS.sleep(3);
System.out.println("Hope You Enjoyed The Program " + this.name);
System.exit(0);
System.out.println("Line Never Reached - Test");
}
}
}


I would use more legible names for variables and classes, they make it easier to read code as plain text (which is useful for other people to understand your code).

For example: naming your class AgeCalculator instead of AgeCalc, or ageCalculator instead of ac when instantiating it.

It is not that important on this example because it's not an extensive code, but if it where, it would be very difficult even for yourself (as the code owner) to remember what all variables mean, for further explanation I would suggest reading the disadvantages paragraph of https://en.wikipedia.org/wiki/Hungarian_notation

Another thing I could suggest you is to have your main method outside AgeCalculator, you should only keep inside methods that represent its behaviour.

I would start practising those, good luck!

EDIT

I found an example at http://www.tutorialspoint.com/java/java_object_classes.htm I modified it a bit to also apply what I understand are more legible names:

import java.io.*;

public class Employee{

String name;
int age;
String designation;
double salary;

// This is the constructor of the class Employee
public Employee(String name){
this.name = name;
}
// Assign the age of the Employee  to the variable age.
public void setAge(int newAge){
age =  newAge;
}
/* Assign the designation to the variable designation.*/
public void setDesignation(String newDesignation){
designation = newDesignation;
}
/* Assign the salary to the variable salary.*/
public void setSalary(double newSalary){
salary = newSalary;
}
/* Print the Employee details */
public void printEmployee(){
System.out.println("Name:"+ name );
System.out.println("Age:" + age );
System.out.println("Designation:" + designation );
System.out.println("Salary:" + salary);
}
}


As you can see, variable names get a bit longer, but they also state what they are for, just by reading their names, so you don't have to remember "What did 'asdf' mean??" xD get me?

Also take into account that in complex code, you probably would have to be remembering many variables, and sometimes it won't be your code at all! So you won't be familiar with it.

Now, the main method, should be in another class (probably another file):

import java.io.*;

public class TheClassThatHasMainMethod{

public static void main(String args[]){
/* Create two objects using constructor */
Employee firstEmployee = new Employee("James Smith");
Employee secondEmployee = new Employee("Mary Anne");

// Invoking methods for each object created
firstEmployee.setAge(26);
firstEmployee.setDesignation("Senior Software Engineer");
firstEmployee.setSalary(1000);
firstEmployee.printEmployee();

secondEmployee.setAge(21);
secondEmployee.setDesignation("Software Engineer");
secondEmployee.setSalary(500);
secondEmployee.printEmployee();
}
}


Because as I said, you should only keep inside a class, methods that represent it's behaviour. At least in Object Oriented Programming, like this case, because each time you initialise a new Object (new Employee), you want it to behave like an Employee and expect it to have Employee methods, like calculating it's salary for example, or getting it's name. But a method called "main" doesn't make sense at all, an employee could give you his salary or name, not his... main?

Also remember that main is a special method that represents the beginning of the execution of your code, it should be separated from the rest of your objects and it must be unique.

Well hope I've helped some, I've also much to learn myself, good luck!

• Thanks for the reply and the advice - Like i said above i am very new to Java coding and wonder what you meant by another class outside AgeCalculator - how would I implement it as code and what would I use it for - like i said not to sure what you mean by "keep inside methods that represent it's behaviour." Aug 10, 2016 at 21:03
• @SwiftYT see EDIT in my answer Aug 11, 2016 at 1:11
• Alot of this is hard for me to still understand but I really understand your talk about naming variables which is great - Secondly the idea of object orientated programming was the structure was shown to me by someone - I was just putting parts of my code into methods and calling them to main - I understand now with the idea of running the code in another class - How do you link the information to one another? Aug 11, 2016 at 10:56
• Object Orientated Programming (OOP, or POO in Spanish) takes some time to get along with, but the idea is to think code as objects, instead of only mathematical functions, purpose is: We humans are much more familiarised with objects, so is easier to think of code as objects. You next step should be focusing on studying OOP in any Language that supports it, Java would be an excellent idea because you already started with it (and I starting my career with that language too!). That would also give you a good general idea on programming (on things like linking code from different files). Aug 11, 2016 at 20:26

• Pick a consistent naming convention in your programming life.
• Give meaningful names for your variables
• Use appropiate Exceptions only when needed
• Method shouldn't know anything beyond its scope (You are directly accessing the scanner in many methods)
• More accessible variables/ methods write at the top of the class. For e.g.: public methods appear above the private methods
• After System.exit(0) nothing will be executed. You are just simply shutting down the JVM.
• Thanks for the reply - I understand the idea of now having more meaningful names - Now what do you mean by appropiate exceptions when needed - If you are talking about the InterruptedException that is always there for the time delay between dialog I believe - Eclipse generates it. You also talk about how the method is accessing the scanner - Should I have scanners per method and they write the data to for instance this.age and this.name - And also you mean just write public methods above private just because? and the system.exit(0) is there to end the program I have no idea how to loop it back Aug 11, 2016 at 10:59
• I used the code to try loop back to main(args) or something along that line but when I did this it - when it got to the user inputs bit it just printed out the questions and did not wait for any input - Maybe its because it already has the inputs - But thats why i went for a method looping it back allowing them to try do it correctly and if false to end program - I see no way of restarting the program inside of it self Aug 11, 2016 at 11:01

(Some comments may seem harsh, especially for your second day of programming. This is not intended to discourage you.)

The thing that sticks out the most to me is you have three ambiguous variables referring to "age".

personAge, age, d_age have nothing to distinguish them in a meaningful way. E.g is "age" not the age of a person? Is there some magical meaning of "d_" that I should be aware of? My first guess while reading was that it would be age in days. I had to guess until i saw exactly how it was used because the name did not give enough information. "personAge" is simple named wrong as it is never set to the age of a person.

The second thing that stood out was all the useless calls to "sleep".

The third thing was the redundant use of "this.", it's a style thing so debatable, but I find it makes the code more cluttered and less readable.

Why are you capitalizing every word in the sentences?

Method names ageCalculate and ageCalculateTwo are misleading and ambiguous. Neither of these methods calculates an age.

System.exit should not be called in a "calculate" method, if at all.

It would probably make more sense in this case to return the value calculated rather than the boolean for success/failure. You could use an exception for failure, or simply accept the idea of a negative result as being valid for what the user wanted to do.

I realize that this is just a simple exercise, but if you were making a "real" application, AgeCalc and your main class would be different. You would have a different class handling the main loop and the I/O. All of these methods are really too trivial to justify a new class in this case though. Here is what I would do:

Create a loop in the main method, so as you mentioned in the comment you can loop until the user choose to exit.

Do all of the I/O in the main method. E.g.:

public static void main(String [] args) {
System.out.println("Input Your Name Before We Start");
String name = s.nextLine();
int age = Math.abs(s.nextInt());
System.out.println("Your Age Is " + age + " Years Old.");
while (true) {
System.out.println("Input The Desired Age To Work Out The Distance To (Use -1 to exit):");
int desiredAge = s.nextInt();
if (desiredAge < age) {
System.out.println("You Cannot Set The Target To Lower Then Your Current Age ("+age+")");
break; // no point in continuing
}

// now do the real work
AgeCalc ac = new AgeCalc();
ac.setCurrentAge(age);
ac.setTargetAge(desiredAge);
System.out.println("You Will Be " + ac.targetAge + " In " + ac.calculateDifference() +" Years");
}
System.out.println("Hope You Enjoyed The Program " + this.name);
}


Call instance methods on your AgeCalc object to set the inputs, do the calculation, and retrieve the result. Display the result in the main loop.

AgeCalc.getInputs() - getting inputs is not the job of the age calculator. It should be receiving inputs. Do the I/O in your main loop and pass the inputs to AgeCalc

The AgeCalc constructor should not be printing a message. Constructors should only construct the object and not have side-effects like that.

Name your variables like: currentAge, targetAge, ageDifference as these are the same terms you are using in the messages ("You Cannot Set The Target To Lower Then Your Current Age"). I prefer Difference to Distance, but either works.

• I love criticism helps me improve and helps me to learn - So i understand that I have age and person age - d_age is distance age - so thats the target they want to work out to. I understand from most comments that I need to start defining variables clearly and also need to set variable names to proper meanings. I use the sleep calls to just put delay between dialog as Im only running it from console - I do not like text being printed so fast. I guess me doing things like Program Restarting and waiting a total of 6 seconds is a bit stupid. I think I have OCD i think personally capitals is neat. Aug 11, 2016 at 14:38
• Yes see I am not sure how to do this but you see I have ageCalculate and ageCalculateTwo also misleading as they are working out the inputs from d_age and age to give the result. I developed these two because I had no idea how to loop back to the start of the program. I tried putting in the main void - the main(args) to start again but it ended up just printing out all my questions and not asking for user inputs. This is the main reason I added system.exits because I have no way asking the user if they want to start again. I used the boolean option as that was the only way Ik to change my main Aug 11, 2016 at 14:42
• So far as I am very new to it - I was going to accept the idea of a negative result as-well as that was something once I first ran it - I noticed I did not like and really did not meet the purpose of the program "Age To Distant Age" not "Age To Past Age" so thank you a lot for all your feed back maybe you can show me examples or link me to tutorials on how I could loop this program instead - Putting my if and else into loops - I will learn java loops soon. Thanks once again @swpalmer Aug 11, 2016 at 14:44