6
\$\begingroup\$

I want to get back into Java programming and so I began to teach myself the language by reading books as well as studying for the OCAJP 8 exam which I plan on taking this coming Monday. I feel as though to get a job it would be nice to have this cert as a fresher and it would also be nice to show I have the ability to write effective code. Since I like exercising I though that an app that would allow me to track my progress would be enjoyable and useful to me.

It's a simple app (3 files) that creates a list of exercises for the user to choose from, input the number of sets, reps, and weight used. The final app will be cleaned up and much more elaborate, but I wanted to see if I could get some input on the following before I went any further. Please keep in mind this was written strictly in notepad so that I can focus on the language itself without the help of an IDE at this point in time...

//My first java workout app

package WorkoutApp;

import WorkoutApp.FirstExerciseClass;

public class FirstWorkoutApp
{
    public static void main(String[] args)
    {
        FirstExerciseClass f1 = new FirstExerciseClass();
        f1.workout();   
        f1.printResults();
    }
}


//My First java workout app

package WorkoutApp;

import java.time.*;
import java.time.format.*;
import java.util.*;
import java.io.File;
import java.io.IOException;
//import java.nio.file.Files;
//import org.apache.commons.io.FileUtils;
import java.io.BufferedWriter;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class FirstExerciseClass
{

// Creates file object and sets pointer
    File filePath = new File("C:\\someFilePath\\Workout Log.txt");

// Gets current day/time information
    private LocalDate date = LocalDate.now();   
// Set format to month/day/year     
    private DateTimeFormatter formatter = DateTimeFormatter.ofPattern("MMMM dd yyyy");
    private String todaysDate = date.format(formatter);

// Declare all variables to be used
    private String exercise;
    private Scanner keyboard = new Scanner(System.in);
    private int sets;
    private int reps;
    private int weight;
    private boolean isFirstExercise;
    private List<String> workoutList = new ArrayList<>();
    private List<String> workoutLog = new ArrayList<>();
    private int totalWeightLifted;
    private List<FirstWorkoutTracker> tracker = new ArrayList<>();


// Adds a list of standard exercises to workoutList
    public void createListOfWorkouts()
    {
        workoutList.add("Squats");
        workoutList.add("Bench Press");
        workoutList.add("Overhead Press");
        workoutList.add("Pull ups");
        workoutList.add("Front Squats");
        workoutList.add("Curls");
        workoutList.add("Shoulder Press");
        workoutList.add("Tricep Extensions");
        workoutList.add("Kettle Bell Swings");
        workoutList.add("Thrusters");
    }

// returns boolean of true if user is planning on doing another exercise otherwise returns false.
    public boolean isStillLifting()
    {
        boolean continueLifting = false;
        System.out.println("Are you done lifting for the day? (y/n)");  
        char result = keyboard.next().charAt(0);
        return continueLifting = (result=='y' || result == 'Y') ? false : true;
    }

// returns boolean of true if user is planning on lifting "today" otherwise returns false
    public boolean isWorkingOut()
    {
        boolean workingOut = false;
        System.out.println("Are you working out today? (y/n)");
        char result = keyboard.next().charAt(0);
        return workingOut = (result=='y' || result == 'Y') ? true : false;
    }

//called to display all workouts that are contained in the workoutList
    public void showWorkoutList()
    {
        int i=1;
        for(String workout : workoutList)
        {
            System.out.println(i++ + ". " + workout);
        }
        System.out.println("0. To Quit");
    }


//Gives user a list of exercise to choose from, based on user input it returns the exercise with a motivational message.
    public String whatExercise()
    {
        System.out.println("Choose an exercise from the list below or press 0 to end your workout.");
        showWorkoutList();
        int exerciseChoice = keyboard.nextInt();
        String exerciseDone = workoutList.get(exerciseChoice-1);
        switch(exerciseChoice)
        {
            case 0:
                System.out.println("We'll start again tomorrow!");
                System.exit(0);
                break;
            case 1:
                System.out.println("Cool, let's do some " + exerciseDone + ".");
                return exerciseDone ;
            case 2:
                System.out.println("Okay aswesome I love " + exerciseDone + "!");
                return exerciseDone;
            case 3:
                System.out.println(exerciseDone + " will get you jacked!");
                return exerciseDone;
            case 4:
                System.out.println(exerciseDone + " sound good!");
                return exerciseDone;
            case 5:
                System.out.println("Let's get some " + exerciseDone + " going!");
                return exerciseDone;
            case 6:
                System.out.println("Can't go wrong with " + exerciseDone + "!");
                return exerciseDone;
            case 7:
                System.out.println("Everyone loves " + exerciseDone + "!");
                return exerciseDone;
            case 8:
                System.out.println("Just a few " + exerciseDone + " will have you swole!");
                return exerciseDone;
            case 9:
                System.out.println("Make sure to have good form on these " + exerciseDone + "!");
                return exerciseDone;
            case 10:
                System.out.println("Get low on these " + exerciseDone + "!");
                return exerciseDone;
            default:
                System.out.println("What was that now?");
                return "";
        }
        return "WHAT?";
    }

//  public List<String> addWorkout(List<String> workouts)
//  {
//      System.out.println("First lets 
//  }

    public int numSets()
    {
        System.out.println("How many sets?");
        int setsDone = keyboard.nextInt();
        return setsDone;
    }

    public int numReps()
    {
        System.out.println("How many reps?");
        int repsDone = keyboard.nextInt();
        return repsDone;
    }

    public int weightUsed()
    {
        int weightLifted = 0;
        System.out.println("At what weight?");
        try
        {
            weightLifted = keyboard.nextInt();
        }
        catch(Exception e)
        {
            System.out.println("Needs to be a number");
            keyboard.next();
            weightUsed();
        }
        return weightLifted;
    }

    public void workout()
    {
        createListOfWorkouts();
        if(isWorkingOut())
        {
        createFile();
            do
            {
                exercise = whatExercise();
                sets = numSets();   
                reps = numReps();
                weight = weightUsed();
                totalWeightLifted += weight;
                tracker.add(new FirstWorkoutTracker(exercise, sets, reps, weight));
                //System.out.println(tracker.get(0));
            }while(isStillLifting());
//      logWorkout(tracker);
        }
        else
        {
            System.out.println("Okay maybe tomorrow.");
        }
    }

    public void printResults()
    {
        System.out.println(todaysDate + "\n");
        for(FirstWorkoutTracker stat : tracker)
        {
            System.out.println(stat);
        }
        System.out.println("\nTotal weight lifted today: " + totalWeightLifted + " pounds!");
    }


    public void createFile()
    {
        boolean b = false;
        if(!filePath.exists())
        {
            try
            {
                b = filePath.createNewFile();
            }
            catch(IOException e)
            {
                e.printStackTrace();
            }
        }
        else
        {
            System.out.println("File already exists, adding to workoutLog.");
        }
        if(b)
        {
            System.out.println("File successfullly created!");
        }
    }

//  public void logWorkout(List<FirstWorkoutTracker> tracker)
//  {
//      try(FileWriter fw = new FileWriter(filePath))
//      {
//      try (BufferedWriter writer = new BufferedWriter(fw))
//      {
//          for(FirstWorkoutTracker stat : tracker)
//          {
//              writer.write(stat.toString());
//          }
//      }
//      }
//  }
}

//My First java workout app

package WorkoutApp;

public class FirstWorkoutTracker
{
    private String exercise;
    private int weight;
    private int reps;
    private int sets;

    public FirstWorkoutTracker()
    {
        this("no lift");
    }

    public FirstWorkoutTracker(String exercise)
    {
        this(exercise, 0);
    }

    public FirstWorkoutTracker(String exercise, int sets)
    {
        this(exercise, sets, 0);
    }

    public FirstWorkoutTracker(String exercise, int sets, int reps)
    {
        this(exercise, sets, reps, 0);
    }

    public FirstWorkoutTracker(String exercise, int sets, int reps, int weight)
    {
        this.exercise = exercise;
        this.sets = sets;
        this.reps = reps;
        this.weight = weight;
    }

    public String toString()
    {
        return (getExercise() + ": " + getNumSets() + " sets of " + getNumReps() + " reps done at " + getWeightUsed() + " pounds.");
    }

    public String getExercise()
    {
        return exercise;
    }

    public int getWeightUsed()
    {
        return weight;
    }

    public int getNumReps()
    {
        return reps;
    }

    public int getNumSets()
    {
        return sets;
    }
}

Questions:

  1. Is the code easily readable, ie. if I were on a team would the way I named the variables and methods make sense to others?
  2. Am I showcasing knowledge and correct usage of things an employer may be looking for in an entry/junior level developer?
  3. Is this a good design for the code?
  4. The good vs bad practices in the code.
  5. Any other advice, comments etc. would be more than welcomed and greatly appreciated!
\$\endgroup\$
3
\$\begingroup\$
// returns boolean of true if user is planning on doing another exercise otherwise returns false.
public boolean isStillLifting()
{
    boolean continueLifting = false;
    System.out.println("Are you done lifting for the day? (y/n)");  
    char result = keyboard.next().charAt(0);
    return continueLifting = (result=='y' || result == 'Y') ? false : true;
}

I prefer JavaDoc style comments, they get picked up by IDE's easier and allow you to break comments into multi-line quicker.

There's a couple things odd about this function...

First, there's no need to declare variables at the top; you can declare them as you need them.

Second, ternaries that consist of condition ? false : true can be turned into !condition.

So, this function could read like so...

// returns boolean of true if user is planning on doing another exercise otherwise returns false.
public boolean isStillLifting()
{
    System.out.println("Are you done lifting for the day? (y/n)");  
    char result = keyboard.next().charAt(0);
    boolean continueLifting = false;
    return continueLifting = !(result=='y' || result == 'Y');
}

Of course, if you look at it now, it seems rather silly to assign false to continueLifting on one line, and then, on the next, to overwrite it without reading the previous value. I'd combine the two...

// returns boolean of true if user is planning on doing another exercise otherwise returns false.
public boolean isStillLifting()
{
    System.out.println("Are you done lifting for the day? (y/n)");  
    char result = keyboard.next().charAt(0);
    boolean continueLifting = !(result=='y' || result == 'Y');
    return continueLifting; 
}

You could choose this way of writing things or you could just directly return the response.

That said...

public boolean isWorkingOut()
{
    boolean workingOut = false;
    System.out.println("Are you working out today? (y/n)");
    char result = keyboard.next().charAt(0);
    return workingOut = (result=='y' || result == 'Y') ? true : false;
}

It's the same... but inverted?

I think you'd be better off by having a askYesNoQuestion method which returns a boolean and then calling it from these two functions...


public String whatExercise()
{
    System.out.println("Choose an exercise from the list below or press 0 to end your workout.");
    showWorkoutList();
    int exerciseChoice = keyboard.nextInt();
    String exerciseDone = workoutList.get(exerciseChoice-1);
    switch(exerciseChoice)
    {
        case 0:
            System.out.println("We'll start again tomorrow!");
            System.exit(0);
            break;
        case 1:
            System.out.println("Cool, let's do some " + exerciseDone + ".");
            return exerciseDone ;
        case 2:
            System.out.println("Okay aswesome I love " + exerciseDone + "!");
            return exerciseDone;
        case 3:
            System.out.println(exerciseDone + " will get you jacked!");
            return exerciseDone;
        case 4:
            System.out.println(exerciseDone + " sound good!");
            return exerciseDone;
        case 5:
            System.out.println("Let's get some " + exerciseDone + " going!");
            return exerciseDone;
        case 6:
            System.out.println("Can't go wrong with " + exerciseDone + "!");
            return exerciseDone;
        case 7:
            System.out.println("Everyone loves " + exerciseDone + "!");
            return exerciseDone;
        case 8:
            System.out.println("Just a few " + exerciseDone + " will have you swole!");
            return exerciseDone;
        case 9:
            System.out.println("Make sure to have good form on these " + exerciseDone + "!");
            return exerciseDone;
        case 10:
            System.out.println("Get low on these " + exerciseDone + "!");
            return exerciseDone;
        default:
            System.out.println("What was that now?");
            return "";
    }
    return "WHAT?";
}

First thing I spot here is the duplication. Cases 1 through 10 are all "print a message, return exerciseDone".

You could have built a List of Strings as well using format strings. String.format will take a format string, and then any values to put into this string. So String.format("Hello, %s", "Bob") will give "Hello, Bob".

Like that, you could have simplified the creation and printing of the Strings by storing them in a list beforehand. The whole use of the switch would fall away in that case.


The question-answers:

  1. Is the code easily readable, ie. if I were on a team would the way I named the variables and methods make sense to others?

It reads easily, but that's by virtue of duplication. Duplication makes it super easy to see what you're doing - because all the code on the screen does the same thing. This isn't really a good point though, because if you make 1 change in one of the values (let's say the user has to enter 9 to exit the program, rather than 0) then the handling of the exit would look just like printing any other string. This would be bad; a reader would think "okay, and more of the same follows" when there's actually a twist.

Other than that, it's pretty readable, it could be better. Let's take a look at another bit of code, where the power of readability is supposed to shine through:

public void workout()
{
    createListOfWorkouts();
    if(isWorkingOut())
    {
    createFile();
        do
        {
            exercise = whatExercise();
            sets = numSets();   
            reps = numReps();
            weight = weightUsed();
            totalWeightLifted += weight;
            tracker.add(new FirstWorkoutTracker(exercise, sets, reps, weight));
            //System.out.println(tracker.get(0));
        }while(isStillLifting());
    //logWorkout(tracker);
    }
    else
    {
        System.out.println("Okay maybe tomorrow.");
    }
}

This is a top level function.

The only thing it is supposed to do is call other functions, which will do the work. Thus, I expect to be able to read it as a recipe.

    createListOfWorkouts();

Great.

    if(isWorkingOut())
    {

Looks good, but ...

    }
    else
    {
        System.out.println("Okay maybe tomorrow.");
    }

It actually makes no sense.

Turns out that you're actually asking the user if they're going be working out.

Well, in that case... ... I think you have two options here. One would be to not ask the user if they're going to be working out at all - that way the check can go - imagine a gym - someone walks in the door, and the gym attendant says...

"Hi, are you going to be working out here today?"

Now, mind you, this is not just any person, right. It's someone with a gym membership. In workout clothing. No, he's just hear to look around- of course he's here for a workout, why else would he show up at the Gym?

If you're gonna be building a logging app or a converter or a calculator or a ... you do not start with "Hello do you want to use this app today". The user ALREADY started the program.

... Anyway, you could also rename the function to askIfUserWillWorkoutToday or something like that. Nothing wrong with descriptive function names. If they get too long, consider simplifying what the function has to do.

Moving on...

createFile();

This ... kind of makes sense, but I'd like to see what kind of file is created here. Logging data? Save data? Temporary storage?

do
{
   ...
}while(isStillLifting());

Hmm... There's a mismatch here. Here you check if we're still lifting, but previously you were talking about workouts. So that's kinda weird. You probably need to rename the function.

exercise = whatExercise();
sets = numSets();   
reps = numReps();
weight = weightUsed();
totalWeightLifted += weight;
tracker.add(new FirstWorkoutTracker(exercise, sets, reps, weight));
//System.out.println(tracker.get(0));

This lot of lines is... okay, but strange. You're obviously building this object, right, FirstWorkoutTracker. I get that. But the variables you're assigning these values to, they're not temporary variables. These values are going somewhere else too. And that's not directly obvious on a first pass. Because I'd have gladly seen all this replaced by

tracker.add(new FirstWorkoutTracker(whatExercise(), numSets(), numReps(), weightUsed()));

That said, perhaps assigning FirstWorkoutTracker to a separate variable would help with keeping the line a bit shorter. On the other hand, those function names need work; weightUsed sounds like a variable name. Actually, so do the other function calls here. Try prefixing with get for anything you're pulling from memory and ask for anything you're going to be asking the user. (Or askFor.) Then you'd get askForNumSets() which clearly shows what the function's gonna do. (Recipes also do this; they'll tell you to cook or bake or braise or stir or fry or ... rather than just hamburger() and voila.)

So this is what I mean - it's pretty readable, you can see what's going on, but there's a few things here and there that could be better.

  1. Am I showcasing knowledge and correct usage of things an employer may be looking for in an entry/junior level developer?

Right now? No. But that's more because programming skill is not really the thing an employer is looking for when it comes to entry level developers. Entry level developers are supposed to be able to learn how to program and quickly at that.

I guess that from a knowledge perspective, there's.... some issues with the design here.

public class FirstExerciseClass

Just doesn't inspire great confidence. The First part worries me that you'd have a SecondExerciseClass lying around somewhere (which would be unheard of). The Class ... that's just redundant. But to call this Exercise would be wrong as well, as this contains much more than just a single Exercise. Really, this is either the Main class or the ExerciseLoggingApplication.

  1. Is this a good design for the code?

See answer 2, but beyond that... the design you have right now, it's okay. It'd be better if the actual IO and the user interface were separated from the logic, but that'd have costed a bunch of time and I don't know the requirements for this code. If the goal is for you to learn something, then it's possible the code is done when it gives a compile error - something which for pretty much any other goal would mean that you're not done yet. So, in this case, is it a bad design because it doesn't allow for easy internationalization, or is it a good design because you did all this so fast, rather than spending several hours on something which was just a learning exercise?

  1. The good vs bad practices in the code.
  2. Any other advice, comments etc. would be more than welcomed and greatly appreciated!

These aren't questions, I guess. See this whole answer.

Overall, you did pretty good; I question some of your methods and program behavior, but if you wrote this to learn then it is fine. I might be a bit harsh here and there; that's so that you can learn, rather than that I say everything is fine when in fact it is not.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ I sincerely appreciate you taking the time to look over and comment on all of the code as well as respond to my questions. I apologize for the delay in my response, but I thank you again for taking the time, and it wasn't harsh at all, I always welcome constructive criticism. That being said, I did put this code together in a short amount of time when I needed a break from studying for the OCAJP 8 exam which I took yesterday, and now that I have some free time I will definitely take into account everything that you've touched on and work to make this app much better. \$\endgroup\$ – JConsorti Aug 23 '16 at 22:42

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.