When a constructor calls a helper function, should the helper mutate the object or return part of the object?

I have a private method that is called from the constructor to instantiate a bunch of objects.

I can do it two ways.

private Monster[,] createMonsters()
    {
        Monster[,] myMonsterArray = new Monster[8, 6];

        for (int i = 0; i < 8; i++)
        {
            myMonsterArray[0, i] = new Monster(SpriteFactory.getMonster1Sprite());
            myMonsterArray[1, i] = new Monster(SpriteFactory.getMonster2Sprite());
            myMonsterArray[2, i] = new Monster(SpriteFactory.getMonster3Sprite());
            myMonsterArray[3, i] = new Monster(SpriteFactory.getMonster1Sprite());
            myMonsterArray[4, i] = new Monster(SpriteFactory.getMonster2Sprite());
            myMonsterArray[5, i] = new Monster(SpriteFactory.getMonster3Sprite()); 
        }

        return myMonsterArray;
    }

called with

monsterArray = createMonsters();

or

private void createMonsters()
    {
        monsterArray = new Monster[8, 6];

        for (int i = 0; i < 8; i++)
        {
            monsterArray[0, i] = new Monster(SpriteFactory.getMonster1Sprite());
            monsterArray[1, i] = new Monster(SpriteFactory.getMonster2Sprite());
            monsterArray[2, i] = new Monster(SpriteFactory.getMonster3Sprite());
            monsterArray[3, i] = new Monster(SpriteFactory.getMonster1Sprite());
            monsterArray[4, i] = new Monster(SpriteFactory.getMonster2Sprite());
            monsterArray[5, i] = new Monster(SpriteFactory.getMonster3Sprite()); 
        }

    }

called with

createMonsters();

Is either better or worse?

As far as I see no-one has mentioned temporal coupling yet, so here it is.

The second one (which returns void) has this code smell. Consider the following constructor code:

createMonsters();
...
doSomethingWithMonsters();

A maintainer easily could mix up the order of the called methods which breaks your class and might turn out only at runtime. It's not possible the following:

Monster[,] monsters = createMonsters();
...
doSomethingWithMonsters(monsters);

Changing the order of the two methods simply would not compile.

Another advantage of the first one is that your field could be final (or readonly, I'm not too familiar with C# but I guess it has something like that). The following constructor code also does not compile in Java if the monsters field is final:

doSomethingWithMonsters(monsters);
monsters = createMonsters();

Furthermore, having the array return value makes the code easier to read. It's obvious from the signature what the method does, readers don't have to check the method body to figure out its side-effects. Finally, you could call createMonsters() from other methods without side-effects.

Anyway, having the separate factory class (as a constructor parameter) looks even better, it's less coupled, +1 to Simon.

See also: Clean Code by Robert C. Martin, G31: Hidden Temporal Couplings, p302

StackExchange Code Review Q#48219, answer score: 16