HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Simplifying a series of type checks and casts in a generic method

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
genericchecksmethodcaststypesimplifyingandseries

Problem

The if/else statements below are not good. How can I improve this method?

public T GetContentByNodeIdSync(Guid nodeId)
{
    var data = m_CMSCatalog.GetContentByNodeId(nodeId);

    if (typeof(T) == typeof(WebFolder))
    {
        var model = (WebFolderDTO)data;
        return Mapper.Map(model);
    }
    else if (typeof(T) == typeof(ContentListItem))
    {
        return Mapper.Map((ContentListItemDTO)data);
    }
    else if (typeof(T) == typeof(Image))
    {
        return Mapper.Map((ImageDTO)data);
    }
    else if (typeof(T) == typeof(File))
    {
        return Mapper.Map((FileDTO)data);
    }
    else if (typeof(T) == typeof(Folder))
    {
        return Mapper.Map((FolderDTO)data);
    }
    else if (typeof(T) == typeof(WebRoot))
    {
        return Mapper.Map((WebRootDTO)data);
    }
    else if (typeof(T) == typeof(Article))
    {
        var model = (ArticleDTO)data;
        return Mapper.Map(model);
    }
    else if (typeof(T) == typeof(WebContent))
    {
        var model = (WebContentDTO)data;
        return Mapper.Map(model);
    }
    return default(T);
}

Solution

You should create a Dictionary that would associate your object's type with their DTO equivalent, so you wouldn't need any if. Then you can use the dictionary to find the types to map. ex :

//This should be instanciated wherever you want, but try to do it only once. 
//The dictionary should be static
dictionary = new Dictionary();
dictionary.Add(typeof(File),typeof(FileDTO));
//etc..

public T GetContentByNodeIdSync(Guid nodeId)
{
    var data = m_CMSCatalog.GetContentByNodeId(nodeId);
    Type dtoType;
    bool mapExists = dictionary.TryGetValue(typeof(T), out dtoType); //To make sure you have an existing map in your dictionary.

    if(mapExists)
        return (T)Mapper.Map(data, dtoType, typeof(T));

    return default(T);
}

Code Snippets

//This should be instanciated wherever you want, but try to do it only once. 
//The dictionary should be static
dictionary = new Dictionary<Type,Type>();
dictionary.Add(typeof(File),typeof(FileDTO));
//etc..

public T GetContentByNodeIdSync<T>(Guid nodeId)
{
    var data = m_CMSCatalog.GetContentByNodeId(nodeId);
    Type dtoType;
    bool mapExists = dictionary.TryGetValue(typeof(T), out dtoType); //To make sure you have an existing map in your dictionary.

    if(mapExists)
        return (T)Mapper.Map(data, dtoType, typeof(T));

    return default(T);
}

Context

StackExchange Code Review Q#60695, answer score: 17

Revisions (0)

No revisions yet.