The constructor ...Don't abuse the constructor

/** 
     * Don't abuse the constructor
     */
    public class Person 
    {
        private final String name;
        private final int age;
        
        public Person(String[] args) 
        {
            this.name = args[0];
            this.age = Integer.parseInt( args[1] );
        }
        
        public Person(byte[] name, Calendar dob) throws UnsupportedEncodingException
        {
            this.name = new String(name, "UTF-8");
            this.age = Years.yearsBetween( new DateTime(Calendar.getInstance()), new DateTime(dob)).getYears();
        }

        /*
         * How do you test that without getting in the constructor mess? 
         */
        public boolean isOlderThan(int age){
        return this.howOldAreYou() > age;
        }
        
        public int howOldAreYou(){
        return this.age;
        }
        
        public String whatsYourName() {
        return this.name;
        }   
    }

    /**
     * It's a pain to test!
     */
    public class PersonTest 
    {
        @Test
        public void newPersonOfArray() throws Exception 
        {
            String name = "Markos";
            int age = 30;
            
            String[] args = {name, String.valueOf(age)};        
            Person person = new Person(args);
            
            Assert.assertEquals(age, person.howOldAreYou());
            Assert.assertEquals(name, person.whatsYourName());
        }
        
        @Test
        public void newPersonOfByteArrayAndCalendar() throws Exception 
        {
            byte[] name = new byte[]{'I', 'n', 'f', 'a', 'n', 't'};     
            Calendar dob = Calendar.getInstance();
            
            Person person = new Person(name, dob);
            
            Assert.assertEquals(0, person.howOldAreYou());
            Assert.assertEquals("Infant", person.whatsYourName());              
        }
    }

    /**
     * Nice and clean
     */
    public class Person 
    {
        private final String name;
        private final int age;
        
        public Person(String name, int age)
        {
            this.name = name;
            this.age = age;     
        }
        
        public boolean isOlderThan(int age){
        return this.howOldAreYou() > age;
        }
        
        public int howOldAreYou(){
        return this.age;
        }
        
        public String whatsYourName() {
        return this.name;
        }   
    }

    public class Persons 
    {
        public static Person valueOf(String[] args)
        {
            String name = args[0];
            int age = Integer.parseInt( args[1] );

        return new Person(name, age);
        }

        public static Person valueOf(byte[] itsName, Calendar dob) throws UnsupportedEncodingException
        {
            String name = new String(itsName, "UTF-8");
            int age = Years.yearsBetween( new DateTime(Calendar.getInstance()), new DateTime(dob)).getYears();
            
        return new Person(name, age);
        }
    }

    /**
     * Smooth and clean testing!
     */
    public class PersonTest 
    {
        @Test
        public void howOldAreYou() throws Exception 
        {
            Person person = new Person(null, 30);
            
            Assert.assertTrue( person.isOlderThan(29) );        
            Assert.assertFalse( person.isOlderThan(30) );       
            Assert.assertFalse( person.isOlderThan(31) );       
        }
    }

It’s a common misconception that since constructors are there to construct an object it’s only resonable to have related code also there. You should do well to keep your constructors for assignments and nothing more. Depending on the amount of “work” done in a constructor you’ll either end up wasting resources when you shouldn’t or deal with some nasty bugs while also making testing much harder.

In this example although a Person is defined by its name and age a String array is passed in instead. Indeed this might well be what you have (through a main method or an input field) and might think that saves you from having to convert it every time you want a Person but think again.

All you really need to construct a Person is a name (String) and an age (int). So even if you do have those, you still need to convert them to a String array only to have the constructor convert them back. That means knowing how the array is constructed, (name at index 0, age at index 1) which is an irrelevant and possible fragile invariant to have on the Person.

In the same manner you force unrelated dependencies (Calendar?) leading to high coupling, making your code harder to understand (how’s a byte array related to a Person?) and sacrificing constructor chaining. You also risk exposing that burden to any classes related to Person. Especially subclasses which don’t have a choice but to accept those constructors.

In most cases all you need is some factory methods that do the conversions for you. That way you’ve effectively seperated conversion and construction and kept your class clean. Depending on how complicated the conversion is you may realise you need a new class altogether, like a parser.

On stackoverlow there is a great answer in case things get really out of hand and you start throwing exceptions from the constructor.

Kudos

Joel Spolsky and Jeff Atwood for their contributtion of StackOverflow.
Yoda Time for an excellent replacement to the abysmal date API.

PS. When creating a String from a byte array make sure you always specify the encoding!

Source

Software Engineer

The constructor ...Don't abuse the constructor

Kudos