Java getters and setters not working

Question

I am doing a homework assignment where I determine the volume of a cylinder. The object of the lesson is Classes and Objects. I have two classes, "CylinderTest" & "Cylinder". Cylinder test calls Cylinder. Everything seems to be working so far except the get and set methods. I am trying to prevent calculations on a negative number, but this is not working, it performs the calculations regardless.

Here is the CylinderTest class

public class CylinderTest
{

    public static void main(String[] args)
    {
        Cylinder myTest = new Cylinder(-1, -1);
        myTest.getHeight();
        myTest.getRadius();
        System.out.println(myTest);

        printHeader();
        double volume = myTest.volume();
        displayCylinder(volume);
    }

    private static void printHeader()
    {
        System.out.println("Cylinder");
        System.out.println("________");
    }

    private static void displayCylinder(double volume)
    {
        System.out.print("Cylinder volume = ");
        System.out.println(volume);
    }
}

Here is the Cylinder class

public class Cylinder
{
    // variables
    public static final double PI = 3.14159;
    private double radius, height, volume;

    // constructor
    public Cylinder(double radius, double height)
    {
        this.radius = radius;
        this.height = height;
    }

    // Volume method to compute the volume of the cylinder
    public double volume()
    {
        return PI * radius * radius * height;
    }

    // accessors and mutators (getters and setters)
    public double getRadius()
    {
        return radius;
    }

    public void setRadius(double radius)
    {
        if (radius > 0.0)
            this.radius = radius;
        else
            this.radius = 1.0;
    }

    public double getHeight()
    {
        return height;
    }

    public void setHeight(double height)
    {
        if (height > 0.0)
            this.height = height;
        else
            this.height = 1.0;
    }

    public double getVolume()
    {
        return volume;
    }

    public void setVolume(double volume)
    {
        this.volume = volume;
    }

}

Answer 1

In your constructor, you need to use the same tests as in getters and setters instead of setting the values directly. Currently, you circumvent the tests in the setter with new Cylinder(-1,-1).

Answer 2

Your constructor should call your setters, and you should check your logic in the setters. Do you really want to carry on with the value of 1 if the calling code passes a negative value?

Answer 3

You could get rid of your constructor and use:

   Cylinder myTest = new Cylinder();
   myTest.setHeight(-1);
   myTest.setRadius(-1);

Or, you could create a "factory" method:

   public static Cylinder createCylinder(double radius, double height)
    {
        Cylinder tmp = new Cylinder();
        tmp.setRadius(radius);
        tmp.setHeight(height);
    }

Though not recommended, syntactically, you could also change your constructor to call the setters.it would look like this:

public Cylinder(double radius, double height)
{
  setRadius(radius);
  setHeight(height);
}

For the reason why this is considered bad practice, see this: Java call base method from base constructor

Answer 4

Further to not executing your tests in the constructor you also don't set the volume (it's null any times).

So, change your constructor to:

public Cylinder(double radius, double height)
{
    this.setRadius(radius);
    this.setHeight(height);
    this.volume = volume();
}

And remove setVolume() and make setHeight() and setRadius() private.

Answer 5

Your setter methods aren't doing the validation because you're not calling them at all. As others have commented, a good idea would be to call them in your constructor instead of assigning values directly to radiusand height.

Initializing the Cylinder's attributes like you did is not incorrect per se. However, since you need to run the "<=0" validation on your input, and your setters already implement this calling them is a simple solution.

A few extra notes that don't affect the result you're looking for but still jumped out to me:

• In TestCylinder, you call both of your getter methods, but you aren't assigning them to anything. Remember that getters return a value, so calling them by themselves effectively does nothing.
• Also in TestCylinder, you call Cylinder.volume() directly, instead of using its getter method getVolume to get the cylinder's volume. Here, I'd recommend either putting the logic to calculate the volume on the getter and using only that method, or having the getter call volume(), in case you need the latter in another part of the Cylinder class.