Chewy
Chewy

Reputation: 85

Logic Error inside my code

I seem to have a logic error within my code. This is because my first Fan Object (FanOne) should display the following output: Speed: 2, Radius: 10.0, Color: yellow.

Instead it has shows Speed: 1

I would think there is a something wrong with my setSpeed() method .. but to me it seems like everything should work out as intended. Please advise, thank you.

public class TestFan {
 public static void main(String[] args) {

 Fan FanOne = new Fan();

 FanOne.fanOn();
 FanOne.setColor("yellow");
 FanOne.setCustomSpeed("MEDIUM");
 FanOne.setCustomRadius(10); 

 System.out.println(FanOne.toString());

System.out.println();

Fan FanTwo = new Fan();

FanTwo.fanOff();
FanTwo.setCustomRadius(5);
FanTwo.setColor("blue");

System.out.println(FanTwo.toString());

 }
}

public class Fan {
 // Declare constant data fields 
 final int SLOW = 1;
 final int MEDIUM = 2;
 final int FAST = 3;

 private int speed;
 private boolean on; 
 private double radius;
 private String color; 

 // Construct a default fan 
 public Fan() {
  speed = SLOW;
  on = false;
  radius = 5;
  color = new String("Blue");
 }

 // Set fan off
 public boolean fanOff() {
  on = false;
  return on;
 }

 // Set fan on
 public boolean fanOn() {
  on = true;
  return on;
 }

 public double getRadius() {
  return radius;
 }

 // Set custom radius 
 public void setCustomRadius(double rad) {
  radius = rad;
 }

 public int getSpeed() {
  return speed;
 }

 // Set custom speed
 public String setCustomSpeed(String speed) {
  if (speed.equals("SLOW")) {
   this.speed = SLOW;  
 } else if (speed.equals("MEDIUM")) {
   this.speed = MEDIUM; 
 } else if (speed.equals("FAST")) {
   this.speed = FAST;
 }
return speed;
}

public String getColor() {
 return color;
}

public void setColor(String colorName) {
 color = colorName;
}

public String toString() {
 if (on == true) {
  return ("Speed: " + speed + ", " + "Radius: " + radius + ", " + "Color: " + color);
 } else {
  return ("Color: " + color + ", " + "Radius: " + radius + ", Alert: " + "The fan is  off!");
 }

 }

}

Upvotes: 0

Views: 64

Answers (2)

Marshall Tigerus
Marshall Tigerus

Reputation: 3764

Your problem is called shadowing. Your code here:

// Set custom speed
 public int setCustomSpeed(String speed) {
  if (speed.equals("SLOW")) {
   speed = String.valueOf(SLOW);  
 } else if (speed.equals("MEDIUM")) {
   speed = String.valueOf(MEDIUM); 
 } else if (speed.equals("FAST")) {
   speed = String.valueOf(FAST);
 }
return speed;
}

is setting the local variable speed equal to 2, leaving the Fan class variable speed equal to whatever it was before (the constructor sets it to 1). Because of this, we need to distinguish the speed variable for the object, using this.speed. This indicates which variable to use.

// Set custom speed
 public String setCustomSpeed(String speed) {
  if (speed.equals("SLOW")) {
   this.speed = String.valueOf(SLOW);  
 } else if (speed.equals("MEDIUM")) {
   this.speed = String.valueOf(MEDIUM); 
 } else if (speed.equals("FAST")) {
   this.speed = String.valueOf(FAST);
 }
return this.speed;
}

To avoid this in the future, you should consider using differently named variables, and read up and understand variable scope and shadowing.

Upvotes: 2

rgettman
rgettman

Reputation: 178263

In your setter method for speed, setCustomSpeed, you are only changing the local String variable speed, not the instance variable speed. It remains unchanged at SLOW, or 1.

Use this to qualify that you mean the instance variable, e.g.

this.speed = SLOW;

It's an int so there is no need to convert your constant to a String here. You can change the other assignment statements accordingly, and you can return this.speed also.

Upvotes: 2

Related Questions