0

When I try to run this code, it won't let me use "bmi". I'm trying to make a simple Body Mass Index calculator, but I don't understand why it won't work. If you could rewrite the code properly, it'd help me learn better.

import java.util.Scanner;

public class Bmi {
    int weight;
    int height;
    int bmi = weight /(height*height) * 703;

    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);
        System.out.print("Enter weight: ");
        int weight = input.nextInt();
        System.out.print("Enter height: ");
        int height = input.nextInt();
        System.out.println(bmi);
   }

}
Arsen Davtyan
  • 1,531
  • 7
  • 20
  • 32
Michael K.
  • 43
  • 9
  • 3
    the variables need to be static since the method is static – Janar Feb 02 '18 at 19:01
  • 3
    Also, `bmi` is currently evaluated once, with the default values of the height and weight (which is zero). You have to calculate it after the user has entered the values. – Andy Turner Feb 02 '18 at 19:02
  • Yup, use `static` or use `new` on the whole BMI class to get an instance of the variable. – markspace Feb 02 '18 at 19:03
  • 1
    @Janar actually, it would be better to define `bmi` as a local variable, and remove the other two. – Andy Turner Feb 02 '18 at 19:03
  • and the same variable names are used in the method which have local scope, which hides the class variables. And using int in the formula will lose the decimal places. – Andrew S Feb 02 '18 at 19:06
  • Here's a tutorial on class (`static`) variables: https://docs.oracle.com/javase/tutorial/java/javaOO/classvars.html vs. instance (not static) variables: https://docs.oracle.com/javase/tutorial/java/javaOO/variables.html Try to understand the difference, it's usually a mistake to mix them. Yes, here you should (probably) be using a third kind of variable, a local variable. Keeping them all straight is a common beginner problem, so just keep pushing through. – markspace Feb 02 '18 at 19:06
  • Better to don't mix read input and calculation logic, start to code in good way from beginning. Check my answer for example. – fxrbfg Feb 02 '18 at 19:43

3 Answers3

4

You are trying to use non static members inside static context.

You don't really need any instance/static variables here. Just go with local variables.

import java.util.Scanner;

public class Bmi {
public static void main(String[] args) {
     Scanner input = new Scanner(System.in);
     System.out.print("Enter weight: ");
     int weight = input.nextInt();
     System.out.print("Enter height: ");
     int height = input.nextInt();
     int bmi = weight /(height*height) * 703;
     System.out.println(bmi);
}
}

Although it is not happen in real world, when your height is greater than weight you end up in zero as integer division is happening. Better you change them to doubles. Give a read Why is the result of 1/3 == 0?

public static void main(String[] args) {   
        Scanner input = new Scanner(System.in);
        System.out.print("Enter weight: ");
        double weight = input.nextInt();
        System.out.print("Enter height: ");
        double height = input.nextInt();
        double bmi = weight / (height * height) * 703;
        System.out.println(bmi);
    }

Finally

int bmi = weight /(height*height) * 703;

That statement won't keep a track on values of weight and height. You need to reevaluate each time when they change.

Suresh Atta
  • 114,879
  • 36
  • 179
  • 284
1

You should never combine the worker and user code in a same class.

Better to create a separate class for calculator say BmiCalculator and define a static method to calculate bmi say calculateBmi. Static because its just dependent on the input it needs and nothing else. Then call and use this static method directly in your CallerClass

Please refer the below code:

import java.util.Scanner;

class BmiCalculator {

 public static double calculateBmi(double weight, double height){
    return weight /(height*height) * 703;
 }

}

public class CallerClass{
  public static void main(String[] args) {
     Scanner input = new Scanner(System.in);
     System.out.print("Enter weight: ");
     double weight = input.nextInt();
     System.out.print("Enter height: ");
     double height = input.nextInt();
     System.out.println(BmiCalculator.calculateBmi(weight,height));
  }
}
Sahil Chhabra
  • 7,481
  • 4
  • 53
  • 53
0

It's because you trying to access instance context from static context. Simpliest way to fix this is add static modificator to your fields and remove bmi calculation from bmi field assignment because all static is initialized when class loaded. More concise way to write this code is that:

    import java.util.Scanner;

    public class Main {

        public static void main(String[] args) {
            Scanner input = new Scanner(System.in);
            System.out.print("Enter weight: ");
            int weight = input.nextInt();
            System.out.print("Enter height: ");
            int height = input.nextInt();
            Bmi bmi = new Bmi(weight, height); //create new bmi instance
            System.out.println(bmi.value());   // get it's value 
            System.out.println(bmi);            // calls toString() method (object text representation) 
//that we already override in Bmi class
        }
    }

We decouple Bmi calculation from user input with way to create separate class, now Bmi class knows nothing about user input and it's good because in this way we increased cohesion and reduce coupling in Main and Bmi classes. It helps to code reuse in different projects and ease mantain efforts. See https://en.wikipedia.org/wiki/GRASP_(object-oriented_design) for more information.

public class Bmi { //place to separate file with name Bmi.java
    private int weight;  
    private int height;

    public Bmi(int weight, int height) {
        this.weight = weight;
        this.height = height;
    }

    /** 
        returns a bmi value
    */
    public int value() {
        return weight / (height  * height) * 703;
    }

    public String toString() {
        return value();
    }

}
fxrbfg
  • 1,548
  • 1
  • 8
  • 16