Java Swing Exercise: BMI Calculator

Question

This is my first question here and this is my working Body Mass Index Swing Calculator. Any improvement or recommendation is welcome.

// Metric units
// Weight in kg
// Height in meters



import javax.swing.*;
import java.awt.*;

public class MainProgram {

    public static void calculateIMC(DataPanel in){
        String salida = in.weight.getText();
        System.out.print(salida);
    }

    public static void main(String[] args) {

        JFrame frame = new JFrame("BMI");

        DataPanel in = new DataPanel();

        frame.setVisible(true);
        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
        frame.setBounds(150,150,500,400);
        frame.setLayout(new GridLayout(0,2));
//        frame.pack();
        frame.add(in);
        System.out.println(Grade.getGrade(33.4));
    }
}

This is an enum example

public enum Grade {
    INS("Insuficient Weight",18.5),
    NRM("Normal Wight",24.9),
    SBR1("Overweight Grade 1",26.9),
    SBR2("Overweight Grade 2: Probesity",29.9),
    OBS1("Obesity Grade 1",34.9),
    OBS2("Obesity Grade 2",3.9),
    OBS3("Obesity Grade 3: Morbid",49.9),
    OBS4("Obesity Grade 4: Extreme",50);

    private final String title;
    private final double lim;

    public String getTitle() {
        return title;
    }

    public double getLim() {
        return lim;
    }

    Grade(String s, double i) {
        title = s;
        lim = i;
    }

    public static String getGrade(double value){
        for (int i=0; i < Grade.values().length;i++){
            if(value > Grade.OBS4.getLim()){
                return Grade.OBS4.getTitle();
            } else if(value < Grade.INS.getLim()){
                return Grade.INS.getTitle();
            } else if(value > Grade.values()[i].getLim() && value < Grade.values()[i+1].getLim()){
                return Grade.values()[i+1].getTitle();
            }
        }
        return null;
    }
}

And the Panel

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class DataPanel extends JPanel {
    JTextField weight = new JTextField(10);
    JTextField height = new JTextField(10);
    JButton calculate = new JButton("Calculate");

    DataPanel(){
        initialize();
    }

    private void initialize() {
        add(new JLabel("Weight: "));
        add(weight);
        add(new JLabel("Height: "));
        add(height);
        add(calculate);

        calculate.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                double w=Double.parseDouble(weight.getText());
                double h=Double.parseDouble(height.getText());
                double bmi=w/(h*h);
                JOptionPane.showMessageDialog(null,"Your BMI is: "+String.format("%.2f",bmi));
                JOptionPane.showMessageDialog(null,"Your Grade is: "+Grade.getGrade(bmi));

            }
        });

        setVisible(true);
        setBorder(BorderFactory.createTitledBorder("BMI"));
//        setBounds();
//        setLayout(new GridLayout(0,2));
        setMaximumSize(new Dimension(30,100));
    }

}

Answer 1

        for (int i=0; i < Grade.values().length;i++){
            if(value > Grade.OBS4.getLim()){
                return Grade.OBS4.getTitle();
            } else if(value < Grade.INS.getLim()){
                return Grade.INS.getTitle();
            } else if(value > Grade.values()[i].getLim() && value < Grade.values()[i+1].getLim()){
                return Grade.values()[i+1].getTitle();
            }
        }

You don't need the first two conditions in the for loop. You can instead say

        if (value > Grade.OBS4.getLim()) {
            return Grade.OBS4.getTitle();
        }

        if (value < Grade.INS.getLim()) {
            return Grade.INS.getTitle();
        }

        for (int i = 1; i < Grade.values().length; i++) {
            if (value > Grade.values()[i - 1].getLim() && value < Grade.values()[i].getLim()) {
                return Grade.values()[i].getTitle();
            }
        }

This is because those two conditions are invariant relative to the loop. They'll either be true the first time or they will never be true. Since you are returning, an else is unnecessary (although harmless if you prefer it).

We don't need to check i equal to 0, as there is no -1 grade. We instead just check the insufficient grade before the loop.

If the BMI grade is exactly equal to the border marker, you'll fall through to the null case. You should make one of those inequalities "or equal to", i.e. >= or <=. Which one depends on the exact rules, which you don't include in your question. I'm going to choose >=.

        for (Grade grade : Grade.values()) {
            if (value < grade.getLim()) {
                return grade.getTitle();
            }
        }

        return Grade.OBS4.getTitle();

You may need to change your limits to match.

Now we don't do any checks outside the loop. And we don't need to compare across grades. We know it's greater than or equal to previous grades, as if it were less than, we would already have returned.

If we fall through the loop, we know that the value represents extreme obesity.

I've added some additional whitespace for readability.

I would prefer to replace Lim with Limit for readability.

You can also use a NavigableMap here. See here for an example on a similar problem.

Answer 2

In your main() function, you create your JFrame and make it visible, and then proceed to set its bounds, its layout and add content to it. This is dangerous behaviour. Swing requires all GUI interaction to be performed on Swing’s Event Dispatching Thread (EDT).

You can improve the safety by creating the JFrame, setting its bounds, its layout, adding content to it, and finally, only after all other configuration has been done, calling frame.setVisible(true);.

But you really should switch to the EDT, using SwingUtilities.invokeLater(...).

public class MainProgram {
    public static void main(String[] args) {
        SwingUtilities.invokeLater( new Runnable() {
            @Override
            public void run() {
                // ... your GUI creation code here, ending with ...
                frame.setVisible();
            }
        });
    }
}

Using lambda syntax makes this easier:

public class MainProgram {
    public static void main(String[] args) {
        SwingUtilities.invokeLater(MainProgram::createGUI);
    }

    private static void createGUI() {
        // ... your GUI creation code here, ending with ...
        frame.setVisible();
    }
}