Generic property disadvantages?

Question

i use generic properties on my project,but i dont know,is there any disadvantage use them,please tell me a scenario,they have a disadvantage?my part of code below.

public class GenericResult<T>
{
    public T Data { get; set; }
    public bool IsSuccess { get; set; }
    public string Message { get; set; }
}

public GenericResult<int> AddCategory(TCategory tCategory)
{
    GenericResult<int> result = new GenericResult<int>();

    //business logic validation,dont make sense,only example :)
    if (tCategory.Name.Lenght > 100)
    {
        result.IsSuccess = false;
        result.Message = "Category Name length is too long";
        result.Data = 0;
    }

    //handle .net runtime error//may be database is not aviable.
    try
    {
        result.Data = this.catalogRepository.AddCategory(tCategory);
        result.IsSuccess = true;
    }
    catch (Exception ex)
    {
        result.Data = 0;
        result.IsSuccess = false;
        result.Message = ex.Message;
    }
    return result;
}

public GenericResult<IEnumerable<TCategory>> GetCategoryHierarchy(TCategory parentCategory)
{
    GenericResult<IEnumerable<TCategory>> result = new GenericResult<IEnumerable<TCategory>>();
    try
    {
        IEnumerable<TCategory> allCategories = catalogRepository.GetAllCategories();
        result.Data = GetCategoryHierarchy(allCategories, parentCategory);
        result.IsSuccess = true;

    }
    catch (Exception ex)
    {
        result.IsSuccess = false;
        result.Data = null;
        result.Message = ex.Message;
    }
    return result;
} 

Answer 1

I'd rather removing IsSuccess and Message and returning only the object. See below...

Take a look at my question Good practices when handling Exceptions in C#. You're returning errors instead of throwing exceptions, and in .NET, that's not suggested.

From MSDN:

Do not return error codes. Exceptions are the primary means of reporting errors in frameworks.

---

What you are doing is suggested in some articles/books I've read, including The Pragmatic Programmer: From Journeyman to Master and this Joel Spolsky article, but as said by MSDN, in .NET exceptions are better for this purpose.

Edit:
If you still want to do it that way (even if that could bring some problems to developers that are working with your code), I think that, in general, it could be a good way. In fact, I'm going to edit the question I linked on this answer to place a link to your code for an alternative of returning errors instead of throwing Exceptions in .NET

Answer 2

If you don't want to throw an exception but prefer to return a result containing either the error or the value i.e. a MayBe that's fine in some situations. But to be honest in this situation I'd prefer simply throwing/passing through the exception.

I'd prefer returning an immutable struct as MayBe instead of a mutable class like you did. It's very similar to Nullable<T>, except it works on reference types and can store an error. Something like:

public struct MayBe<T>
{
    private T value;
    private Exception error;

    public bool HasValue{get{return error==null;}}
    public T Value
    {
      if(error!=null)
        throw error;
      else
        return value;
    }

    public static MayBe<T> CreateError(Exception exception)
    {
      return new MayBe<T>(default(T),exception);
    }

    public static MayBe<T> CreateValue(T value)
    {
      return new MayBe<T>(value,null);
    }

    public static implicit operator MayBe<T>(T value)
    {
        return CreateValue(value);
    }

    public override string ToString()
    {
        if(HasValue)
            return "Value: "+Value.ToString();
        else
            return "Error: "+Error.GetType().Name+" "+Error.Message;
    }
}

Your code becomes

public MayBe<int> AddCategory(TCategory tCategory)
{
    try
    {
       return this.catalogRepository.AddCategory(tCategory);
    }
    catch (Exception ex)
    {
        return MayBe<int>.CreateError(ex);
    }

    return result;
}

public MayBe<IEnumerable<TCategory>> GetCategoryHierarchy(TCategory parentCategory)
{
    try
    {
        IEnumerable<TCategory> allCategories = catalogRepository.GetAllCategories();
        return allCategories;

    }
    catch (Exception ex)
    {
        return MayBe<int>.CreateError(ex);
    }

    return result;
}

One problem I see with this implementation is that exceptions are not completely immutable. That can cause problems if the same MayBe<T> throws on multiple threads. Perhaps someone can suggest a better implementation.

Answer 3

If your application scope is completely inside .NET scope, then this pattern is of no use and just as others have mentioned, you should let exceptions be thrown and you might want to change the exceptions.

However, if your application scope is wide that might include any other client side framework, probably via JSON, web services etc, and if client side framework does not properly support exceptions then this pattern may be useful. For example, in JSON based Javascript call, you will always expect a result, and a message indicating a failure on server side or not. Failure on client side could be either failure on server side or network failure, usually all client framework will detect and only report network failures and improperly coded framework will lead to chaos when you will not get any error report on client side of what exactly failed on server side.

One more place this pattern is very useful is, when you are writing some plugin in the UI or inside someone else's framework where just throwing exceptions can result in undesired results as after having exceptions, third party framework may say "Unexpected error" as they do not and they are not made to understand your exceptions. This pattern is useful while being inside someone else's framework and still letting underlying framework work correctly regardless of your failure. And you probably can communicate correctly within your app framework.

I recently have seen, and its still a bug, WPF stops processing some pending UI related activities if you set a source of an image that is a web address and that does not exist. You will see a network related exception traced, but WPF will incompletely stop processing anything that was in pending tasks and app still works but it does affect other UI elements where it should not.

Answer 4

The use of the automatic property is quite fine and I do not see any issues.

But I strongly discourge the pattern using a class as a result to tell the outside world that something failed. Return null or throw an exception when something badly fails.

hth

Mario