CODEWITHSUNDEEP
javavariablesconstructor
user3186023
user3186023

Reputation:

Java confusion with parameter passing in constructors

I'm creating a BMR calculator, and I have a class called User. This class contains all the methods used to calculate the BMR, as well as a constructor to pack the user's data (age, gender, weight, height) together.

The code is:

public class User {

int age;
String gender; // todo: use an Enum
double height; // height stored in cm, weight in kg (so if user enters in feet/lbs, conversions are done to cm/kg and *THEN* passed through to constructor below)
double weight;
double activityMultiplier; // todo: use an Enum  (possibly)
int bmr;


public User(int age, String gender, double height, double weight,
    double activityMultiplier) {
    this.age = age;
    this.gender = gender;
    this.height = height;
    this.weight = weight;
    this.activityMultiplier = activityMultiplier;

    bmr = calcBMR();
}

/**
 * If user input is correct, this method will calculate the BMR value of the user given their input and measurement choices.
 * 
 * @param None
 * @return BMR Value
 */
public int calcBMR() {
    int offset = gender.equals("M") ? 5 : -161;
    // This is the body of the calculations - different offset used depending on gender. Conversions to kg and cm done earlier so no conversions needed here.
    // The formula for male and female is similar - only the offset is different.
    return (int) (Math.round((10 * weight) + (6.25 * height) - (5 * age) + offset)); // This is the Miffin St-Jeor formula, calculations done in cm/kg
    }

/**
 * If the user selects the TDEE option, this method will be executed after the calcBMR() method. 
 * A value from the calcBMR() method will be passed down to this method, and is multiplied
 * by the activity level parameter passed into this method.
 * 
 * @param bmr (output from calcBMR() method
 * @return TDEE Value
 */
public int calcTDEE(int bmr) {
    return (int) Math.round(calcBMR() * activityMultiplier);
}

}

My concern is that I'm not sure that the way I'm initialising the value for bmr in the constructor (bmr = calcBMR()) is correct. I can't calculate the bmr till the users age, gender, height and weight are recorded and stored in variables (which are what the 5 lines above do). Is this programming structure okay? I.e. when a User object is created, the age, gender, height and weight are stored in variables, and THEN a method is called within the constructor to calculate and store another value.

Is there a better way to do this? If not, do I need to do this.bmr = calcBMR() or is bmr = calcBMR() fine?

Note the User object is created in a seperate class. The reason I'm mainly confused is because I'm not passing a bmr parameter into the constructor, I'm using a method return value to initiate the value of an instance variable instead.

Upvotes: 2

Views: 747

Answers (3)

Akash Thakare
Akash Thakare

Reputation: 22972

Better to separate the calculation from the construction.

I personally believe we should avoid doing this. According to me for any person using Class which is constructing your Object will be unaware of the fact that bmr is also being calculated in the User by seeing this particular constructor, until he look into the code. When you are building API you should make it more readable for the consumer who will going to use your User, apart from that it's totally valid. Moreover, as suggested by Jeff Storey if you don't want user Objects to use your calcBMR you should make it private.

One more point I want to add here that bmr is one of the Health related parameter of the Person so you should collect all this kind of parameters in separate class which can include bodyFat, bmr, medicalAge etc in it and pass your User Object in constructor of this class. So, ultimately Health parameters can only be constructed with valid user only.

Upvotes: 0

Kenny Tai Huynh
Kenny Tai Huynh

Reputation: 1599

I think your way is ok. However, it is better if you use all your properties is "private" and have get/set for them. So, when another class extends your class, we don't have to worry about messy things. With the calcTDEE method, I think you just need to put your bmr property inside it because it is set in your constructor. So, at the time you call this method, the bmr has the right value.

public int calcTDEE() {
    return (int) Math.round(bmr * activityMultiplier);
}

Hope this help.

Upvotes: 0

Jeff Storey
Jeff Storey

Reputation: 57192

It's ok syntactically, however you should not call an override-able (public/protected non-final) method from your constructor. If someone overrides it, it could mess up the construction of your object. Calling helper methods from a constructor is fine, just make it private or final.

this.bmr = calcBMR() is the same as bmr = calcBMR()

Saying bmr.calcBMR() wouldn't make sense because the calcBMR method is on the User object. bmr is an int so it doesn't have a method named calcBMR

Whether you use this or not is a matter of your preference. It only really makes a difference if you have a local variable also named bmr, and then you are explicitly calling the instance variable rather than the local. In general, it's confusing to have a local and instance variable with the same name.

Your calcTDEE method is a little bit off though. You can just use the value of bmr, not pass it in or recalculate it, so it would be

public int calcTDEE() {
    return (int) Math.round(bmr * activityMultiplier);
}

Upvotes: 3

Related Questions