Sunday, May 17, 2015

Singleton pattern

Singleton pattern... People love it or hate it. There are also group of people who don't mind them.

I'm still try figure out which I'm :] I'm for sure not the one who love them. I'm also cannot say that I rally hate them because I still use them in code (It is like with my Facebook account. After I created one I try not complain about fb. ). So probably I'm best fit to don't mind group.

But my past experience showing me that I have bigger tendency to removing them from code than adding new one. And probably some colleagues from work will be happy about this because we spend a lot of time discussing about problem.

I remove them because I put even more effort in good design of systems. In a lot of cases thanks to changes I just don't need global objects. Which is good. It allow me to better utilize multithreading thanks to encapsulation.

Today sadly I had problem where I still don't know cleaner solution than global state:

//////////////////////////////////////////////////////////////////////////
template<> bool write<CResHandle>( ISerializerWritera_writerconst CResHandlea_value )
{
    return a_writer.serialize(a_value.isValid() ? a_value->getId() : wrResourceID());
}

//////////////////////////////////////////////////////////////////////////
template<> bool read<CResHandle>( ISerializerReadera_readerCResHandlea_value )
{
    wrResourceID id;

    if (!a_reader.deserialize(id)) return false;

    if (CResource::isValid(id))
    {
        a_value = getResMgr()->createResource(id);
    }
    else
    {
        a_value = nullptr;
    }

    return true;
}

Function getResMgr() return global resource manager. I don't want to pass it around inside arguments because this is ugly and I would need to do the same with each manager I want to use. This would increase my arguments list and each new manager would recommend changes in all write/read functions.
I thought maybe about storing inside ISerializerReader/ISerializerWriter function like setData()/data() in some Qt classes. This way it will be really easily to extend list of available data even by game (which may add some new write/read functions). 

I'm still not sure about this solution and how nice it is. I will spend some time thinking about it but for now I will leave getResMgr() use with nice macro call right before it : 

WR_TODO("gwojciechowski""Think about way to not use global getResMgr().");

If you have any idea, thoughts or you just want to comment this. I'm open on opinion how to make this nice and clean :]

Greg 

4 comments:

  1. Hey Greg, it might not be the nicest solution but you could extend functionality of CResource::isValid static function this way:

    bool CResource::isValid(const wrResourceID &id, /*out*/ CResHandle **ppHResource)

    As you can deduce it would validate the resource ID and if it's valid it will create and return CResHandle ptr.

    What you think?

    Cheers!

    ReplyDelete
    Replies
    1. Hi Łukasz, I don't think that this solve the source of the problem. Function isValid is static method which just checking if a_resourceID is equal to InvalidID.

      With solution you propose if I want to allow for more than one Resource Manager (I don't know usage of this yet but let's assume I'm crazy enough to do this) then I can not explicit pass to function the manager that I want to use :/

      For me: singletones, global functions, static members, static variables all of them are put into the same group: global state. And my experience showing that in multithreaded environment you don't want to have a lot of them.

      Cheers!

      Delete
    2. Yeah, right. I was a little bit out of track cause you've mentioned singleton in the beginning of your post, thus I've assumed you want to have only one resource manager at runtime. Ok then, let me continue with a question: are those template functions read/write global? Cause they look so in your snippet.

      Delete
    3. Yeah you are right title may be a little bit misleading. Functions are global and have simple template form:

      template bool write( ISerializerWriter& a_writer, const T& a_value )
      template bool read( ISerializerReader& a_reader, T& a_value )

      ISerializerReader/ISerializerWriter are interfaces for different implementations of storing/reading data (Binary/XML).

      Delete