I'm trying to figure out if I'm doing superclasses right. Also, I welcome critique on Java structure/syntax etc.
Animal.java:
public class Animal{
private static int counter = 0; // how many animals we have created (to demonstrate a static variable)
private Boolean isWild = true; // is the animal wild?
private Boolean hasSharpPointyTeeth = false; // does it have sharp, pointy teeth?!
private int age = 0; // how old
Animal(){ // default constructor
this.isWild = isWild;
this.hasSharpPointyTeeth = hasSharpPointyTeeth;
this.age = age;
counter++;
}
Animal(Boolean isWild, Boolean hasSharpPointyTeeth, int age){
this.isWild = isWild;
this.hasSharpPointyTeeth = hasSharpPointyTeeth;
this.age = age;
counter++;
}
//begin getters/setters
public void setIsWild(Boolean isWild){
this.isWild = isWild;
}
public void setAge(int age){
this.age = age;
}
public void setHasSharpPointyTeeth(Boolean hasSharpPointyTeeth){
this.hasSharpPointyTeeth = hasSharpPointyTeeth;
}
public Boolean getIsWild(){
return isWild;
}
public Boolean getHasSharpPointyTeeth(){
return hasSharpPointyTeeth;
}
public int getAge(){
return age;
}
public static int getCounter(){
return counter;
}
//end getters/setters
public String speak(){
return "Grrrr!";
}
public String toString(){
String out = speak() + "\n" +
"Age: " + getAge() + "\n" +
"Wild? " + getIsWild() + "\n" +
"Has Sharp Pointy Teeth? " + getHasSharpPointyTeeth() + "\n";
return out;
}
}
Dog.java
public class Dog extends Animal{
private String name = "None";
private String owner = "None";
private String breed = "Unknown";
private String sex = "Unknown";
Dog(){
super(false, false, 0); // call the constructor from the Animal class
this.name = name;
this.owner = owner;
this.breed = breed;
this.sex = sex;
}
Dog(Boolean isWild, Boolean hasSharpPointyTeeth, int age, String name, String owner, String breed, String sex){
super(isWild, hasSharpPointyTeeth, age); // call the constructor from the Animal class
this.name = name;
this.owner = owner;
this.breed = breed;
this.sex = sex;
}
//begin getters/setters
public void setName(String name){
this.name = name;
}
public void setOwner(String owner){
this.owner = owner;
}
public void setBreed(String breed){
this.breed = breed;
}
public void setSex(String sex){
this.sex = sex;
}
public String getName(){
return name;
}
public String getOwner(){
return owner;
}
public String getBreed(){
return breed;
}
public String getSex(){
return sex;
}
//end getters/setters
@Override
public String speak(){
return "Woof!";
}
@Override
public String toString(){
String out = super.toString() + // call the toString method of Animal, not the overridden one in Dog
"Name: " + getName() + "\n" +
"Breed: " + getBreed() + "\n" +
"Sex: " + getSex()+ "\n" +
"Owner: " + getOwner() + "\n";
return out;
}
}
Test.java
public class Test{
public static void main(String[] args){
Dog fluffy = new Dog(false,true,3,"Fluffy","Ted","Westie","Male");
Animal rabbit = new Animal(true,true,8);
System.out.println("We made " + Animal.getCounter() + " animals. Here's their info:");
System.out.println();
System.out.println(fluffy);
System.out.println(rabbit);
}
}
1 Answer 1
private Boolean isWild = true; // is the animal wild?
private Boolean hasSharpPointyTeeth = false; // does it have sharp, pointy teeth?!
Why are you using the wrapper class? Use the primitive boolean
instead.
Also, about comments:
private static int counter = 0; // how many animals we have created (to demonstrate a static variable)
private Boolean isWild = true; // is the animal wild?
private Boolean hasSharpPointyTeeth = false; // does it have sharp, pointy teeth?!
private int age = 0; // how old
All the comments here are unnecessary with better variable names. Comments explain why, not what.
All of the variable names are fine, maybe except counter
: could be numberOfAnimals
...
NAMING
public Boolean getIsWild()
should be isWild()
. Likewise,
public Boolean getHasSharpPointyTeeth()
should be hasSharpPointyTeeth()
.
But getAge()
, for example, should stay getAge()
.
public static int getCounter()
should be getNumberOfAnimals()
speak()
is poorly named. I would've expected it to print what it says, but it just returns what it says. I would either change it to do what I said, even though I would suggest against it, or rename it. I would suggest rename it to getSpeech()
.
Misc
this.isWild = isWild;
this.hasSharpPointyTeeth = hasSharpPointyTeeth;
this.age = age;
In the default constructor is completely unnecessary: it already has those values, does it need those same values again? Same in the Dog
class:
this.name = name;
this.owner = owner;
this.breed = breed;
this.sex = sex;
The begin and end getters/setters comments are unnecessary: why are those there?
The (){
after each method should be () {
according to Java standard conventions.
Constructors should be public
.
I think the sex
property should be Animal
's.
Also you should check the parameters and see if they are valid: you don't want age to be set to -1
or breed
to be set to something like hello
... do you? I'll leave that up to you, but the general format is this:
if (is not valid) {
throw new IllegalArgumentException("Why is it not valid?);
}
You shouldn't be testing with main()
. Instead use JUnit 4 or any similar testing library.
Final Code
public class Animal {
private static int numberOfAnimals = 0;
private boolean isWild = true;
private boolean hasSharpPointyTeeth = false;
private int age = 0;
private String sex = "Unknown";
public Animal() {
numberOfAnimals++;
}
public Animal(String sex, boolean isWild, boolean hasSharpPointyTeeth, int age) {
this.sex = sex;
this.isWild = isWild;
this.hasSharpPointyTeeth = hasSharpPointyTeeth;
this.age = age;
numberOfAnimals++;
}
public void setIsWild(Boolean isWild) {
this.isWild = isWild;
}
public void setAge(int age) {
this.age = age;
}
public void setHasSharpPointyTeeth(Boolean hasSharpPointyTeeth) {
this.hasSharpPointyTeeth = hasSharpPointyTeeth;
}
public void setSex(String sex) {
this.sex = sex;
}
public Boolean isWild() {
return isWild;
}
public Boolean hasSharpPointyTeeth() {
return hasSharpPointyTeeth;
}
public int getAge() {
return age;
}
public String getSex() {
return sex;
}
public static int getNumberOfAnimals() {
return numberOfAnimals;
}
public String getSpeech() {
return "Grrrr!";
}
public String toString() {
String out = getSpeech() + "\n" + "Age: " + getAge() + "\n" + "Wild? "
+ isWild() + "\n" + "Has Sharp Pointy Teeth? "
+ hasSharpPointyTeeth() + "\n";
return out;
}
}
public class Dog extends Animal {
private String name = "None";
private String owner = "None";
private String breed = "Unknown";
public Dog() {
super("Unknown", false, false, 0);
}
public Dog(boolean isWild, boolean hasSharpPointyTeeth, int age, String name,
String owner, String breed, String sex) {
super(sex, isWild, hasSharpPointyTeeth, age);
this.name = name;
this.owner = owner;
this.breed = breed;
}
public void setName(String name) {
this.name = name;
}
public void setOwner(String owner) {
this.owner = owner;
}
public void setBreed(String breed) {
this.breed = breed;
}
public String getName() {
return name;
}
public String getOwner() {
return owner;
}
public String getBreed() {
return breed;
}
@Override
public String getSpeech() {
return "Woof!";
}
@Override
public String toString() {
String out = super.toString()
+ "Name: " + getName() + "\n" + "Breed: " + getBreed() + "\n"
+ "Sex: " + getSex() + "\n" + "Owner: " + getOwner() + "\n";
return out;
}
}
-
1\$\begingroup\$ @h3rrmiller Please try not to accept answers instantly; wait a couple of hours or days for the best answer. \$\endgroup\$TheCoffeeCup– TheCoffeeCup2015年10月26日 22:42:40 +00:00Commented Oct 26, 2015 at 22:42
-
\$\begingroup\$ I'm pretty active on the unix site so I'll see other answers trickle in and accept them if they're better. But thanks for the tip \$\endgroup\$h3rrmiller– h3rrmiller2015年10月26日 22:43:30 +00:00Commented Oct 26, 2015 at 22:43
-
\$\begingroup\$ Just FYI, you didn't add sex into the constructor of your revised Animal class. Nice review though \$\endgroup\$RHok– RHok2015年10月26日 22:52:15 +00:00Commented Oct 26, 2015 at 22:52
-
\$\begingroup\$ @RicHo Thanks for the catch, didn't notice it. \$\endgroup\$TheCoffeeCup– TheCoffeeCup2015年10月26日 22:52:59 +00:00Commented Oct 26, 2015 at 22:52
-
\$\begingroup\$ @MannyMeng my really hurts to see such a long list of parameters in
Dog
constructor and it smells too. Must be a code smell, check this thread. Ideally it should be somestruct
but in Java we can pass someenum
, wdyt? \$\endgroup\$vivek– vivek2015年10月29日 02:55:55 +00:00Commented Oct 29, 2015 at 2:55