BMR (Basal Metabolic Rate) Calculator

Been working on a BMR calculator on and off for a while now. Created an interface and been over different ways to store user input, different ways to calculate BMR/TDEE etc.

I created another class called User for the calculations (as suggested by someone). This way I can pack the data of a user together with any methods that use the data.

Any criticism is appreciated. Thanks

```
/**
* @author "Faizan Tahir"
* @title "BMR Calculator"
* A program to calculate a users BMR (Basal metabolic rate) and TDEE (Total Daily Energy Expenditure).
* Allows user to select how they would like to input their information (e.g. weight in kg or cm),
* Calculates users TDEE given an activity level. Basic and straightforward to use. User does not need to convert
* to their standard unit of measure, this calculator does that for them.
* Future updates: Allow user to save their information so as to track fitness progress
**/

import javax.swing.*;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.BorderLayout;
import java.awt.FlowLayout;
import java.util.HashMap;

public class BmrMain extends JFrame {

// Declarations for gender/weight/height and other variables
HashMap activityMap;
String[] activityLevels = {"Sedentary", "Lightly Active", "Moderately Active", "Very Active", "Extra Active"};

int userAge;
String userGender;
double userHeight;
double userWeight;
double userActivityLevel;

public BmrMain(String title) {

// Main JFrame
setTitle(title);
JPanel mainPanel = new JPanel();

// All JPanel declarations
JMenuBar menuBar = new JMenuBar();
JPanel imgPanel = new JPanel();
JPanel agePanel = new JPanel();
JPanel genderPanel = new JPanel();
JPanel heightPanel = new JPanel();
JPanel weightPanel = new JPanel();
JPanel weightHeightPanel = new JPanel(new BorderLayout());// combines genderPanel, agePanel, heightPan

JFrame mainFrame;

This variable is never used and can be deleted.

JPanel mainPanel;

You don't need this to be an object field (sometimes called an instance field). It's enough for it to be a local variable. I.e.

mainPanel = new JPanel();

could be

JPanel mainPanel = new JPanel();

This is true of most of your object fields.

The advantage of keeping it to a local variable is that it reduces the scope and visibility. Note that if someone adds another class to the same package they would be able to see and change mainPanel with the original definition. This can get confusing, which is why we have scope and visibility limitations.

As a rule of thumb, you should try to limit the scope as narrowly as possible.

I would initialize the variable in the declaration unless there is some reason why it might need to be initialized to different values. E.g. different constructors or based on a method parameter. Note that if you are initializing a collection variable (e.g. a Map), you will have to add the elements separately in many cases.

private  JTextField ageField;

This is a little different. Because you use it in the interior method, you have to do something to make it persist beyond the scope of a local variable. Making it an object field does this. Another alternative would be to make

ageField = new JTextField(4);

into a final variable.

final JTextField ageField = new JTextField(4);

If you do keep it as an object field, private is the correct visibility.

bmrValuePanel = new JPanel();
        tdeeValuePanel = new JPanel();

These too are never used and can be removed.

// Activity level map
        activityMap = new HashMap();
        activityMap.put("Sedentary", 1.2);
        activityMap.put("Lightly Active", 1.375);
        activityMap.put("Moderately Active", 1.55);
        activityMap.put("Very Active", 1.725);
        activityMap.put("Extra Active", 1.9);

Rather than using a comment to name this, consider creating a method

private void initializeActivityValues() {
        activityValues = new HashMap();
        activityValues.put("Sedentary", 1.2);
        activityValues.put("Lightly Active", 1.375);
        activityValues.put("Moderately Active", 1.55);
        activityValues.put("Very Active", 1.725);
        activityValues.put("Extra Active", 1.9);
    }

This also has the benefit of making the original method shorter, particularly if you do the same thing with other sections.

I also changed the name. You may be able to do better. I didn't try to figure out what the numbers mean.

StackExchange Code Review Q#102557, answer score: 2