Skip to main content
The 2024 Developer Survey results are live! See the results
added 648 characters in body
Source Link
Roland Illig
  • 21.3k
  • 2
  • 33
  • 83

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.

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 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.

Source Link
Roland Illig
  • 21.3k
  • 2
  • 33
  • 83

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.