What would you change in this program? (Java)

Question

 public class Zoo{
    public static void main (String[] args){
        Animal animal1 = new Animal ("Giraffe", 5, 'M');
        Animal animal2 = new Animal ("Lion", 10, 'F');

        System.out.println (animal1.getName () + " " + animal1.getAge () + " " + animal1.getGender ());
        System.out.println (animal2.getName () + " " + animal2.getAge () + " " + animal2.getGender ());
    }


 }
    class Animal{
        private int age;
        private char gender;
        private String name;

        public Animal(){
            this.name = "Giraffe";
            this.age = 5;
            this.gender = 'M';
        }

        public Animal (String name, int age, char gender){
            this.name = name;
            this.age = age;
            this.gender = gender;
        }

        public void setName (String name){
            this.name = name;
        }

        public String getName (){
            return name;
        }

        public void setGender (char gender){
            this.gender = gender;
        }

        public char getGender(){
            return gender;
        }

        public void setAge (int age){
            this.age = age;
        }

        public int getAge (){
            return age;
        }
    }

Would you consider this code correct in general and to show the concept of encapsulation? Thanks :)

Answer 1

One problem, you should not repeat your System.out.println code. You should have a toString method in your Animal class instead.

Another problem. You Zoo should probably have a list of animals, rather than explicitly create them yourself.

Try to make these changes, and if you have problems, I will give you some more advice. I hesitate to give the actual code solution, because this looks like homework to me.

Answer 2

You could consider adding validation to ensure that an Animal cannot be put in an invalid state. i.e. throw an IllegalArgumentException if someone tries to set a negative age, etc.

Answer 3

It's student code.

A couple of suggestions:

1. I'd make Gender an enumeration. I can pass any string into your constructor as written.
2. I'd override equals, hashCode, and toString in your Animal. Google for "Joshua Bloch Chapter 3" to find out how.
3. What, no inheritance? That would seem to be the student classic.

Answer 4

Animal that cannot fly, walk? Zoo would require suitable methods too.

The object model is http://en.wikipedia.org/wiki/Anemic_Domain_Model. (This is a homework, so does not matter)

Answer 5

It looks like it'll work, but you're not really encapsulating anything, since all your fields are exposed via setters - you could call animal1.setGender('x') or animal1.setName(null), leaving the object in an invalid state. Of course, you could say "fine, I won't ever pass in 'x' or null" but this doesn't scale when you're working in a team, as everyone has different assumptions about what constitutes "reasonable" expectations (for example, is gender case-sensitive?). Worst case is that without validation, every time a new piece of code is added that calls getGender() or getName() it must either handle bad return values or risk breaking if the value isn't something the author expected.

The point of encapsulation is that you are adding a layer between the raw fields and external code - this layer is responsible for ensuring the object is in a valid state, and for performing operations that require additional knowledge of the object's internals (such as Brian's date-of-birth suggestion - I like that).

As others have posted, a good start would be adding validation to setters (negative age/invalid name) and use enum for gender (or constants + validation, if you must).

Answer 6

How likely is it that this animal will change gender ? Name ? perhaps. Age ? Certainly. But I'd reduce the amount of mutability in your code. This will make your code more reliable (particularly in the face of multithreading, but it's a good practice).

I would certainly check for invalid inputs. What's a valid age ? What's a valid name ? (is a blank string acceptable?). Your animal object should be validating such settings, since it will contain such concepts of validity.

Perhaps in place of age, I would give your Animal a date-of-birth (again, immutable). Then when you ask that animal for its age, it can calculate it directly.

A good guideline is, get your objects to do things for you. Don't set individual attributes etc. when your object can derive such stuff.

Answer 7

Among other comments above, I would think that Animal is a sufficiently important class to be public(open to discussion) and reside in its own file.

That would separate your 'client' program or executable code (your Zoo class with its main), from your business logic (You only have an Animal object now, but you are bound to have some more in your application if you grow it, in which case I would start thinking about separating classes in packages too).

Answer 8

Bloch, J., 2008. Effective Java. 2nd ed:

Item 15: Minimise mutability: Classes should be immutable unless there's a very good reason to make them mutable. Immutable classes are easier to design and implement, are less prone to error and more secure. They are inherently thread-safe and can therefore be shared freely. To make the Animal class immutable you would:

1. Remove the "setter" methods.
2. Make the class final so it can't be extended.
3. Make all fields final.
4. Make all fields private.

Answer 9

Also make sure you reuse your real constructor by the parametereless one:

    /**
     * Constructs a default <tt>Animal</tt>. Current implementation
     * constructs a 5 years old male Giraffe.
     */
    public Animal() {
        this("Giraffe", 5, 'M');
    }

    public Animal (String name, int age, char gender){
        this.name = name;
        this.age = age;
        this.gender = gender;
    }