Eliminate incomplete constructors

[Quintus]

At some parts SMC classes have constructors that do not actually build a complete object. Instead, they rely on the calling code to actually complete the object. Take for example the constructor of cOverworld:

cOverworld :: cOverworld( void )
{
    m_sprite_manager = new cWorld_Sprite_Manager( this );
    m_animation_manager = new cAnimation_Manager();
    m_description = new cOverworld_description();
    m_layer = new cLayer( this );

    // ...

    m_next_level = 0;

    m_player_start_waypoint = 0;
    m_player_moving_state = STA_STAY;
}

This code is incomplete. Why? It constructs an instance of cOverworld_description and does not initialize it. Why does this matter? Because cOverworld_description’s constructors does something entirely undocumented:

cOverworld_description :: cOverworld_description( void )
{
    m_path = "world_1";
    m_name = _("Unnamed");
    m_visible = 1;
    m_user = 0;

    m_comment = _("Empty");
}

It defaults to loading the world world_1! The consequence of this is that if you create an instance of cOverworld, you are forced to post-initialize it by manually changing the chosen overworld afterwards, but before you call any method on your new object. Check out what cOverworld_Manager::Load_Dir() does:

                // ...
                overworld = new cOverworld();

                // set directory path
                overworld->m_description->m_path = fs::path(current_dir);
                // default name is the path
                overworld->m_description->m_name = path_to_utf8(current_dir.filename());
                // set user
                overworld->m_description->m_user = user_dir;

                objects.push_back( overworld );

                overworld->Load();
            }

Do you see how this first constructs the new cOverworld instance, then post-initializes its m_description member, and then calls the Load() method on the object? This is very bad code quality as it violates one of the main principles of object-oriented programming, the principle of secrecy. As the caller, I do not need to bother on how to create an object, I do not need to cater for an objects internals. I call it’s class’ constructor (potentially with arguments), and have a ready-to-use object.

This code needs to be wiped out entirely and replaced with proper constructors that take arguments to build the instances the way they need to be built.

Valete.

[Quintus]

Note that for the specific case of cOverworld, this has been fixed by adding a proper constructor for loading from a world directory in 085f7f8. However, I’m sure there are more occurences of this.