This is simple example, which I used to elaborate my question.

private void checkValue(string templateId)
{
    if (templateId == "CASH_CHECK")
    {
        //do somthing
    }
    else if (templateId == "CHECK_RETURN" || templateId == "DEPOSIT_MONEY")
    {
        //do somthing
    }
}

You can see in the above example I'm checking the templateId value with separate string values. Suppose If I have to do this templateId comparison in the whole application and one day DEPOSIT_MONEY value will be changed to DEPOSIT_FOREIGN_MONEY and I will have to change this value in the whole application. And I also think it steers clear of code readability. therefor I supposed to use struct like this.

struct Templates
{
    public const string CASH_CHECK = "CASH_CHECK";
    public const string CHECK_RETURN = "CHECK_RETURN";
    public const string DEPOSIT_MONEY = "DEPOSIT_MONEY";
}

Then It will use something like this,

private static void checkValue(string templateId)
{
    if (templateId == Templates.CASH_CHECK)
    {
        //do somthing
    }
    else if (templateId == Templates.CHECK_RETURN || templateId == Templates.DEPOSIT_MONEY)
    {
        //do somthing
    }
}

I just want to know, is this the good approach to handle this? or what is the most appropriate way to handle this?

CodePudding user response：

Yes, it's good to have a central location for strings that will be used for comparisons that can be referenced throughout the code. You're previous approach is often referred to as "magic strings" and are considered to be bad because for a few reasons including:

1. They can be duplicated throughout the code making updates difficult
2. You don't have much context about what they are for (a simple variable name can inform the viewer about what the string relates to)
3. You may happen to use the same string in multiple places for different purposes. If you used a find and replace to update them, intending to update the string for one purpose, you would inadvertently update it everywhere whereas two variables with the same value would allow you update either one independently.

CodePudding user response：

To add, instead of using raw string, better wrap them in a Template type. Reason: You can do better type checks, rule out invalid/empty values. And you no more deal with strings, except during initialization.

//e.g. using record
public record Template(string Value);

Then you can do,

class Templates
{
    public Template CASH_CHECK { get; } = new Template("CASH_CHECK");
    public Template CHECK_RETURN { get; } = new Template("CHECK_RETURN");
    public Template DEPOSIT_MONEY { get; } = new Template("DEPOSIT_MONEY");
    public Templates(){    }

    private void CheckValue(Template template)
    {
        if (template == CASH_CHECK)
        {
            //do somthing
        }
        else if (template == CHECK_RETURN || template == DEPOSIT_MONEY)
        {
            //do somthing
        }
    }
}

Another benefit is that since Template is a Type now, you can introduce additional properties or Behaviours. That way, Your CheckValue can be free from conditions,

public record Template(string Id, Action Do);

struct Templates
{
    public Templates() { }

    private readonly ISet<Template> _knownTemplates = new HashSet<Template>
    {
        new Template("CASH_CHECK", () => { } /*do something*/),
        new Template("CHECK_RETURN", () => { } /*do something else*/),
        new Template("DEPOSIT_MONEY", () => { } /*do something else*/)
    };

    private void CheckValue(string templateId)
    {
        _knownTemplates.First(t => t.Id == templateId).Do();
    }
}

This is just one of the ways how you can remove conditions.