4
\$\begingroup\$

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);
    }
}
\$\endgroup\$
6
\$\begingroup\$
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;
    }

}
\$\endgroup\$
  • 1
    \$\begingroup\$ @h3rrmiller Please try not to accept answers instantly; wait a couple of hours or days for the best answer. \$\endgroup\$ – TheCoffeeCup Oct 26 '15 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 Oct 26 '15 at 22:43
  • \$\begingroup\$ Just FYI, you didn't add sex into the constructor of your revised Animal class. Nice review though \$\endgroup\$ – RHok Oct 26 '15 at 22:52
  • \$\begingroup\$ @RicHo Thanks for the catch, didn't notice it. \$\endgroup\$ – TheCoffeeCup Oct 26 '15 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 some struct but in Java we can pass some enum, wdyt? \$\endgroup\$ – vivek Oct 29 '15 at 2:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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