Refactoring Real World Application

In this post, I’d like to share with you a bit of refactoring.

Class name: ExternalAccessService

Methods to refactor:

1. GenerateTokenForApply
2. GenerateTokenForResults
3. GenerateTokenForClient

We don’t need all details of these methods but what is interesting about them is that every one of them is generating token. And the way they do that is very similar:

//1. method
var result = _repository.GenerateTokenIdForApplyUrn(agentToken, urn);
if (!string.IsNullOrEmpty(result))
{
    return new ResultStatus() { Success = true, StringValue = result};
}
return UnableToGenerateToken();

//2.method
var result = _repository.GenerateTokenIdForResults(agentToken, requestId, clientId);
if (!string.IsNullOrEmpty(result))
{
    return new ResultStatus() { Success = true, StringValue = result };
}
return UnableToGenerateToken();


//3.method
var result = _repository.GenerateTokenForClient(agentToken, clientId);
if (!string.IsNullOrEmpty(result))
{
    return new ResultStatus() { Success = true, StringValue = result };
}
return UnableToGenerateToken();

Lots of rows of code that do the same thing with different parameters. So I thought that it is a great opportunity for refactoring.

And my result is here:

private ResultStatus GenerateToken(Func<string> func)
{
    string result = func.Invoke();
    if (!string.IsNullOrEmpty(result))
    {
        return new ResultStatus() { Success = true, StringValue = result };
    }
    return UnableToGenerateToken();
}

And from 6 lines of code in every method, we have only one:

//1. method
return GenerateToken(() => _repository.GenerateTokenIdForApplyUrn(agentToken, urn));
//2. method
return GenerateToken(() => _repository.GenerateTokenIdForResults(agentToken, requestId, clientId));
3.method
return GenerateToken(() => _repository.GenerateTokenForClient(agentToken, clientId));

However don’t be mistaken, refactoring is not just about reducing number of lines of code. It is also about re-thinking usage of your code. Re-using your existing code, etc.

Another example of refactoring is ResultStatus class.

Class name: ResultStatus

Property to refactor: Value

Before refactoring, the class was used to return status from service layer of application.

Example of class:

public class ResultStatus
{
    private List errors = new List(); 

    public bool Success { get; set; }

    public string stringValue { get; set; }

    public int intValue { get; set; }

    public List Errors {
        get { return errors; }
        set { errors = value; }
    }
}

Evidently for every different type of value property, we would have to add another property with that type. For example, public decimal decimalValue , etc.

Solution to this problem is refactioring and using C# Generics.

After refactoring:

public class ResultStatus<T>
{
    private List<string> errors = new List<string>(); 

    public bool Success { get; set; }

    public T Value { get; set; }

    public List<string> Errors {
        get { return errors; }
        set { errors = value; }
    }
}

Now, we don’t have to use different properties to get value. We will just initialize our class with type that we expect:

return new ResultStatus<string>() { Success = true, Value = result };

Update:

After Alexandru Lungu pointed that it would be nice to have more context to ResultStatus<T>().

Solution in my application:

private ResultStatus<string> GenerateToken(Func<string> func)
{
    string result = func.Invoke();
    if (!string.IsNullOrEmpty(result))
    {
        return new ResultStatus<string>() { Success = true, Value = result };
    }
    return UnableToGenerateToken();
}
 
private ResultStatus<int> GenerateToken(Func<int> func)
{
    int result = func.Invoke();
    return new ResultStatus<int>() {Success = true, Value = result};
}

public ResultStatus<string> GenerateTokenForSomeAction()
{
    /*bussines logic code */
    return GenerateToken(() => _repository.GenerateTokenForSomeAction("test", "test"));
}
 
public ResultStatus<int> GenerateNumericToken()
{
    /*bussines logic code */
    return GenerateToken(() => _repository.GenerateTokenForSomeAction2("test", "tes"));
}

Reason that I choose to create 2 GenerateToken functions is because I have only string and int token. And also different type of validations for them.

In my case, I had to choose to create some sort of switch statement in a general function or create 2 specific functions.

In case you would need a more general approach, I would suggest to use the function that Alexandru suggested to me:

private ResultStatus<T> GenerateToken<T>(Func<T> func)
{
    T result = (T) func.Invoke();
    return new ResultStatus<T>() { Success = !result.Equals(default(T)), Value = result };
}

This way, you will get a truly generic function.

Summary

As you can see, sometimes it's about compromise between a general and more specific solution. It is always best to think of both and choose the one that suits you more. Sometimes, it is difficult to say but hey that's why we have such a great communities as on CodeProject where you can ask questions to difficult problems you are facing.

That’s it. Hope you liked my examples and I hope you will also refactor your applications and make them better and more readable for yourself and your colleagues.