2
\$\begingroup\$

Create a Java class named HeadPhone to represent a headphone set. The class contains:

  • Three constants named LOW, MEDIUM and HIGH with values of 1, 2 and 3 to denote the headphone volume.

  • A private int data field named volume that specifies the volume of the headphone. The default volume is MEDIUM.

  • A private boolean data field named pluggedIn that specifies if the headphone is plugged in. The default value is false.

  • A private String data field named manufacturer that specifies the name of the manufacturer of the headphones.

  • A private Color data field named headPhoneColor that specifies the color of the headphones.

  • A private String data field named headPhoneModel that specifies the Model of the headphones.

  • getter and setter methods for all data fields.

  • A no argument constructor that creates a default headphone.

  • A method named toString() that returns a string describing the current field values of the headphones.

  • A method named changeVolume(value) that changes the volume of the headphone to the value passed into the method

Create a TestHeadPhone class that constructs at least 3 HeadPhone objects. For each of the objects constructed, demonstrate the use of each of the methods.

Here is what I have thus far....

import java.awt.Color;

public class HeadPhones {
    public static final int LOW = 1;
    public static final int MEDIUM = 2;
    public static final int HIGH = 3;

    private int volume;
    private boolean pluggedIn;
    private String manufacturer;
    private Color headPhoneColor;

    public HeadPhones() {
        volume = MEDIUM;
        pluggedIn = false;
        manufacturer = "";
        headPhoneColor = Color.BLACK;
    }
    /**
     *
     * @return volume of headphone
     */
    public int getVolume() {
        return volume;
    }
    /**
     * set volume of headphone
     *
     * @param volume
     */
    public void setVolume(int volume) {
        this.volume = volume;
    }
    /**
     *
     * @return true if the headphone is plugged in, false otherwise
     */
    public boolean getPluggedIn() {
        return pluggedIn;
    }
    /**
     * set plugged in
     *
     * @param pluggedIn
     */
    public void setPluggedIn(boolean pluggedIn) {
        this.pluggedIn = pluggedIn;
    }
    /**
     *
     * @return manufacturer
     */
    public String getManufacturer() {
        return manufacturer;
    }
    /**
     * set manufacturer
     *
     * @param manufacturer
     */
    public void setManufacturer(String manufacturer) {
        this.manufacturer = manufacturer;
    }
    /**
     *
     * @return headphone color
     */
    public Color getHeadPhoneColor() {
        return headPhoneColor;
    }
    public String getColorName() {
        String colorName = "Black";
        if (headPhoneColor == Color.BLACK || headPhoneColor == Color.black) {
            colorName = "Black";
        } else if (headPhoneColor == Color.WHITE || headPhoneColor == Color.white) {
            colorName = "White";
        } else if (headPhoneColor == Color.RED || headPhoneColor == Color.red) {
            colorName = "Red";
        } else if (headPhoneColor == Color.PINK || headPhoneColor == Color.pink) {
            colorName = "Pink";
        } else if (headPhoneColor == Color.CYAN || headPhoneColor == Color.cyan) {
            colorName = "Cyan";
        } else if (headPhoneColor == Color.BLUE || headPhoneColor == Color.blue) {
            colorName = "Blue";
        } else if (headPhoneColor == Color.GREEN || headPhoneColor == Color.green) {
            colorName = "Green";
        } else if (headPhoneColor == Color.GRAY || headPhoneColor == Color.gray) {
            colorName = "Gray";
        }
        return colorName;
    }
    /**
     * set headphone color
     *
     * @param headPhoneColor
     */
    public void setHeadPhoneColor(Color headPhoneColor) {
        this.headPhoneColor = headPhoneColor;
    }
    /**
     * returns a string describing the current field values of the headphone
     */
    public String toString() {
        StringBuilder s = new StringBuilder("(");
        s.append("manufacturer = ").append(manufacturer).append(", ");
        s.append("volumne = ").append(volume).append(", ");
        s.append("plugged in = ").append(pluggedIn).append(", ");
        s.append("color = ").append(getColorName()).append(")");
        return s.toString();
    }
}

And this is my test class....

import java.awt.Color;

public class TestHeadPhones {
    public static void main(String[] args) {
        HeadPhones[] headphones = new HeadPhones[3];

        headphones[0] = new HeadPhones();
        headphones[0].setHeadPhoneColor(Color.CYAN);
        headphones[0].setManufacturer("Thinksound");
        headphones[1] = new HeadPhones();
        headphones[1].setManufacturer("Monster");
        headphones[1].setHeadPhoneColor(Color.white);
        headphones[1].setPluggedIn(true);
        headphones[1].setVolume(HeadPhones.HIGH);
        headphones[2] = new HeadPhones();
        headphones[2].setManufacturer("Sennheiser");
        headphones[2].setHeadPhoneColor(Color.ORANGE);
        headphones[2].setPluggedIn(true);
        headphones[2].setVolume(HeadPhones.LOW);
        for (int i = 0; i < 3; i++) {
            System.out.println("Headphone #" + (i + 1));
            System.out.println("toString() results: " + headphones[i]);
            System.out.println("getVolume() results: " + headphones[i].getVolume());
            System.out.println("getPluggedIn() results: " + headphones[i].getPluggedIn());
            System.out.println("getManufacturer() results: " + headphones[i].getManufacturer());
            System.out.println("getHeadPhoneColor() results: " + headphones[i].getColorName() + "\n");
        }
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ FYI, you are missing the field for the model. \$\endgroup\$
    – cbojar
    Commented Sep 15, 2018 at 3:04

3 Answers 3

1
\$\begingroup\$

Since most of the class is just getters and setters, there isn't much substantial to comment on.

The biggest problem I see is the getColorName method. It's doing unnecessary checks, and I usually reach for a Map in cases like this anyways. Map lookups are faster for for this, and are a little neater in my opinion. For this case, it would look like:

Map<Color, String> colorToName = new HashMap<>();

colorToName.put(Color.BLACK, "Black");
colorToName.put(Color.WHITE, "White");
colorToName.put(Color.PINK, "Pink");
colorToName.put(Color.CYAN, "Cyan");
colorToName.put(Color.BLUE, "Blue");
colorToName.put(Color.GREEN, "Green");
colorToName.put(Color.GRAY, "Gray");

Then, to get the name, you'd do:

colorToName.get(Color.PINK); // Returns "Pink"

You'll probably want to initialize colorToName as a static member of the class so it's not being recreated constantly.


Notice how I'm ignoring the lowercase variants. That's because, if you look at the source:

/**
 * The color pink.  In the default sRGB space.
 */
public final static Color pink      = new Color(255, 175, 175);

/**
 * The color pink.  In the default sRGB space.
 * @since 1.4
 */
public final static Color PINK = pink;

They refer to the same thing, so checking both is redundant.

\$\endgroup\$
2
  • \$\begingroup\$ Why when I go to an online IDE to execute the code, does it say, "Error: Main method not found in class HeadPhones, please define the main method as: public static void main(String[] args)"? \$\endgroup\$
    – Tony D
    Commented Sep 14, 2018 at 22:39
  • \$\begingroup\$ @TonyD You may have typed it wrong. I can't say without seeing it. It must differ than what you have here, since the IDE is looking in class HeadPhones for the main, but you have it in TestHeadPhones here. And Code Review is for working code. You didn't mention originally that the code isn't working. \$\endgroup\$ Commented Sep 14, 2018 at 22:45
1
\$\begingroup\$

This is a pretty straightforward exercise, and you did a fine job.

Criticisms of the requirements (not your code):

  • Fields headPhoneColor and headPhoneModel are verbose and redundant. Why not just color and model?
  • Why changeVolume(value) rather than setVolume(value)? I think that your setVolume(value) is better.
  • I would think that certain characteristics are immutable once the object is instantiated: manufacturer, color, and model. Perhaps those fields should not have setters.

Criticisms of your code:

  • getPluggedIn() is a predicate that returns a boolean. By convention, it should be called isPluggedIn().
  • Do these headphones go up to 11? It might be a good idea to perform some validation in setVolume(volume), and throw an IllegalArgumentException if needed.
  • The getColorName() function could use some work. The String colorName = "Black" assignment is redundant and should therefore be avoided. Interestingly, the Color.toString() documentation says that you should not rely on it, but I'd use it as a fallback anyway, because it is possible to create colors outside the predefined palette.

    A switch block could work:

    public String getColorName() {
        switch (this.headPhoneColor) {
            case Color.BLACK: case Color.black: return "Black";
            case Color.WHITE: case Color.white: return "White";
            …
            default: return this.headPhoneColor.toString();
        }
    }
    
  • In toString(), "volumne" is a misspelling.
  • Technically, you don't have to write the constructor explicitly, since a zero-parameter public constructor is provided implicitly.

    public class HeadPhones {
        public static final int LOW = 1;
        public static final int MEDIUM = 2;
        public static final int HIGH = 3;
    
        private int volume = MEDIUM;
        private boolean pluggedIn;
        private String manufacturer = "";
        private Color headPhoneColor = Color.BLACK;
    
        public int getVolume() {
            return volume;
        }
    
        …
    }
    

    That said, your explicit constructor is not necessarily bad.

\$\endgroup\$
2
  • \$\begingroup\$ Why when I go to an online IDE to execute the code, does it say, "Error: Main method not found in class HeadPhones, please define the main method as: public static void main(String[] args)"? \$\endgroup\$
    – Tony D
    Commented Sep 14, 2018 at 22:39
  • \$\begingroup\$ Because your main() method is in TestHeadPhones, not HeadPhones. \$\endgroup\$ Commented Sep 14, 2018 at 22:40
1
\$\begingroup\$

Your code looks good, but the requirements look quite old-fashioned.

Since September 2004, symbolic constants like the volume in your example are no longer defined as int but rather as enum:

public enum Volume {
    OFF, LOW, MEDIUM, HIGH
}

Using an enum is safer than an int because the users of your class cannot accidentally set the volume to -17 or to 12345. The only invalid value is null, and you can easily defend against that:

public void setVolume(Volume volume) {
    this.volume = Objects.requireNotNull(volume);
}

In your code you have written Javadoc comments /** … */ for some of the methods. Comments are an important thing for any reader of your code. The comments should contain all the details that cannot be easily seen by looking at the definition of the method.

Currently your comments only repeat words from the method definition. This information is useless and should be removed. Instead, in the comment for setVolume(int), you should mention that only the values LOW, MEDIUM and HIGH are allowed and what happens when someone calls the method with an invalid value.

Until you learn about if and throw statements, the comment should just say "don't do that". In some weeks you should throw an exception for the invalid values and update the comment accordingly.


Your toString method looks quite complicated right now. Many people use that style, but there is a nice way:

public String toString() {
    return String.format(
        "Headphone %s by %s, %s, volume %s",
        model,
        manufacturer,
        pluggedIn ? "plugged in" : "unplugged",
        volume);
}

This style makes it easy to see the general pattern of the generated string. Each %s is a placeholder that is replaced with the values in the lines below, in the given order.

The ?: operator is useful for generating human-readable text instead of the very technical true/false.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.