Animal choir simulator

Question

I got a PHP developer interview test to solve for a company. I didn't get the job but I would like to know where I was wrong. Did I understand the test completely?

/**
 * Create an Animal Choir simulator
 *
 * The task constraints are:
 *
 * There must be 3 different choir member animals
 * (i.e. dogs, cats, mice)
 *
 * Every animal must have a sing method that returns a string representation of a voice
 * (i.e. 'bark', 'meow', 'squeak')
 *
 * Every animal must have a loudness property which can have 3 settings,
 * depending on which the sing method result will be rendered as
 * lowercase, first letter uppercase and uppercase.
 *
 * Singer groups are groups of animals with the same loudness property value.
 * Singer group song is represented with a CSV of the group singer sing result in random order.
 *
 * The choir simulator must have implement the following methods:
 *    crescendo - the choir start singing from the least loud singer group, and then are being joined 
 *                by more and more loud singer groups until they are singing all together.            
 *                The joining is represented with a new line.
 *                Example:
 *                  meow, squeak, bark
 *                  Meow, bark, squeak, Bark, meow
 *                  bark, Meow, MEOW, squeak, BARK, meow, Bark
 *
 *    arpeggio  - the choir singer groups of the same loudness start singing one by one from
 *                the least loud to the loudest
 *                Example:
 *                  meow, squeak, bark
 *                  Meow, Bark
 *                  MEOW, BARK
 *
 */

//TODO: Describe the class hierarchy


//Choir class
class Choir{    

    public $line_ending = '';
    public $line_separator = '';
    public $voices = '';

    public function crescendo(){
        $crescendo_song = '';

        $animals = new Animal();        
        //We define which animal voices will be included in song
        $animals->voices = $this->voices;

        //First we start with the silent
        $animals->loudness = 'silent';
        //Call the function     
        $silent = $animals->sing();
        //echo the result
        $crescendo_song .= $this->stringForm($silent);

        //Then we continue with the normal, but also merge with silent
        $animals->loudness = 'normal';      
        $normal = array_merge($animals->sing(),$silent);
        $crescendo_song .= $this->stringForm($normal);

        //And in the end we merge loud with silent and normal
        $animals->loudness = 'loud';    
        $loud = array_merge($animals->sing(),$silent,$normal);
        $crescendo_song .= $this->stringForm($loud);

        return $crescendo_song;
    }


    public function arpeggio(){
        $arpeggio_song = '';

        $animals = new Animal();        
        //We define which animal voices will be included in song
        $animals->voices = $this->voices;

        //First we start with the silent
        $animals->loudness = 'silent';  
        $arpeggio_song .= $this->stringForm($animals->sing());

        //Then normal
        $animals->loudness = 'normal';      
        $arpeggio_song .= $this->stringForm($animals->sing());

        //And then the loud
        $animals->loudness = 'loud';
        $arpeggio_song .= $this->stringForm($animals->sing());

        return $arpeggio_song;

    }


    private function stringForm($array){
        //Randomize the array
        shuffle($array);
        //Predefine a song and a separator
        $song = '';
        $comma = '';
        //Form song string
        foreach ($array as $slog) {
            $song .= $comma.$slog;  
            $comma = $this->line_separator.' ';                 
        };
        return $song.$this->line_ending;
    }


}

//Animal class
class Animal{
    public $loudness = '';

    public function sing(){
        $song = array();
        foreach ($this->voices as $voices) {
            switch ($this->loudness) {
                case 'silent':
                    array_push($song,strtolower($voices));
                break;          
                case 'normal':
                    array_push($song,ucfirst($voices));
                break;
                case 'loud':
                    array_push($song,strtoupper($voices));
                break;
                default:
                    array_push($song,$voices);
                break;
            }
        }   
        return $song;
    }

}


$choir = new Choir();
$choir->line_ending = PHP_EOL;
//$choir->line_ending = '<br>'; //For cleared viewing in browser use <br> tag
//Define the line separator for CSV
$choir->line_separator = ',';
//Define the voices of animals
$choir->voices = array('bark','meow','squeak');
//Call and echo the functions
echo $choir->crescendo();
echo $choir->arpeggio();

Answer 1

Let's start with class hierarchy. The choir is made up of the animals.

The Choir class should have a list of its Animal members:

$felix = new Cat();
$rex = new Dog();
$choir = new Choir(array($felix, $rex, $bernand));

Then each animal has a different way of singing but they all sing . That is classic polymorphism. (In fact they sing in such a similar way that the sing() method goes into the Animal class).

class Dog extends Animal {
    public function  __construct() {
        $noise = 'bark';
        ....

class Animal {
    private $loudness;
    private $loudnessLevels = array('silent', 'normal', 'screaming');
    private $noise;

    public function __set($name, $value) {
        if($name == 'loudness') {
            if(!in_array($value, $loudnessLevels) {
                throw new Excpetion('Unknown loudness');
            }

            $loudness = $value;
     ...

For a discussion about the value of getter/setters in PHP (coming from a C++/Python background I'm pleased never having to do PHP OOP).

Now for some (e.g. I didn't check for bugs) low-level criticism:

• Use getters and setters
• In your sing() function there is the default case - you just output the string in this case but actually it is an undefined case and should throw an exception.

Answer 2

Unfortunately you seem to have misunderstood or missed this part of the brief:

There must be 3 different choir member animals (i.e. dogs, cats, mice)

Every animal must have a sing method that returns a string representation of a voice

So it sounds like they want it so you can add a mouse to the choir and it will only squeak, and a cat will only meow. Instead you have a single choir which randomly picks a sound to make and no distinct objects for the animals. I suspect they were wanting an Animal to make a single noise which is specified upfront in a more object-orientated way. Something like this at a minimum:

class /* enum */ Animals
{
    const CAT = 'meow';
    const DOG = 'bark';
    const MOUSE = 'squeak';
}

class /* enum */ Loudness
{
    const LOUD = 'loud';
    const NORMAL = 'normal';
    const SILENT = 'silent';
}

class SingingAnimal
{
    private $noise, $loudness;

    public function __construct($noise, $loudness)
    {
        $this->noise = $noise;
        $this->loudness = $loudness;
    }

    public function arpeggio()
    {
        //Implementation here
    }

    public function crescendo()
    {
        //Implementation here
    }

    public function sing()
    {
        //Implementation here
    }
}

class Choir
{
    public /* SingingAnimal[] */ $singers = array();

    public function addAnimal($animal, $loudness)
    {
        $this->singers[] = new SingingAnimal($animal, $loudness);
    }
}

$choir = new Choir();
$choir->addAnimal(AnimalNoises::CAT, Loudness::SILENT);
$choir->addAnimal(AnimalNoises::MOUSE, Loudness::LOUD);
$choir->addAnimal(AnimalNoises::DOG, Loudness::NORMAL);

Though even this is pretty sloppy, depending on the amount of time you have I'd probably go deeper and write out the standard interfaces and abstract classes they're looking for with these tests: interface Animal, abstract class Enum (for all the validation methods), class LoudnessEnum extends Enum, class AnimalEnum extends Enum, etc.