Best practice for constant string for implementations to use

Question

Say I have an interface:

public interface IFeature
{
    Task execFeature();
}

and two implementations:

public class FirstFeature : IFeature
{
    private IWebApi webApi;
    public FirstFeature(IWebApi webApi)
    {
        this.webApi = webApi;
    }

    public async Task execFeature()
    {
        string response = await webApi.getClassName();
        IResult result;
        if(response==null)
            result = new TextResult("Error accessing api - check internet connection/api address");
        else
            result = new TextResult("Hello dear user – the selected class name is " + response);
        result.display();
    }
}

public class SecondFeature : IFeature
{
    private IWebApi webApi;
    public SecondFeature(IWebApi webApi)
    {
        this.webApi = webApi;
    }

    public async Task execFeature()
    {
        List<string> classNames = new List<string>();
        var classNameTasks = Enumerable.Range(1, 3).Select(i => webApi.getClassName()).ToArray();
        classNames.AddRange((await Task.WhenAll(classNameTasks)));
        IResult result;
        if (classNames[0] == null)
            result = new TextResult("Error accessing api - check internet connection/api address");
        else 
            result = new TextResult("Hello dear user – we’ve selected three new class names for you, and they are " + classNames[0] + ", " + classNames[1] + ", and " + classNames[2]);
        result.display();
    }
}

As you can see, in both implementations I had to do the result = new TextResult("Error accessing api - check internet connection/api address"); line to report the error.

What is the best practice in OOP/Good Design to have a constant error_string that I can access in all of my implementations?

the way it is right now, code is duplicated.

Answer 1

I don't think there is a best practice. It is just a matter of preference.

I store constants inside static classes.

public static class Constants
{
   public static class Messages
   {
      public const string Error = "Error accessing api...";
      public const string Hello = "Hello ...";
   }
}

Usage

var result = new TextResult(Constants.Messages.Error);

FYI: Some developers prefer Enum.

Answer 2

I usually make a distinction based on the intended audience for the message. As such, I break them into two categories.
Regardless of the category, I avoid writing the same code more than once (e.g. message strings).

Developer Messages

• Messages displayed in unit tests, messages displayed only during debugging, or messages logged to super-detailed diagnostic files
• Developer messages require no localization and should be written in the language of the development house.
  For example, if the development company is located in Moscow, then there's a strong argument to write developer messages in Russian.
  In practice, a lot of developers choose English.

• Implementation options are multiple. I typically keep it simple, using fields in a static class. Note that you could have a message class for each type that will display messages, or you could have a central message class where you group multiple classes. You could have nested message groups. You could also add other types of constants for use in your code... As I mentioned, options and preferences abound.

  public static class FeatureMessages
  {
      #region Public Fields
  
      public const string ApiAccessError = 
          @"Error accessing api - check internet connection/api address";
      public const string SelectedClassFormatString = 
          @"Hello dear user – the selected class name is {0}";
  
      #endregion
  }
  

User Messages

• Messages displayed to end users. For example, installation prompts, in the user GUI menus, on a user warning message boxes, etc.
• These messages should be localized.
• Implementation is simple, and will require at least a default language resource file. There are lots of resources out there that can show you how to do this, you can see a simple explanation here

Answer 3

I would generally recommend using a resource file (created from your project settings). You may want to provide some kind of wrapper if you want to make this code more testable.

Answer 4

Put that in a static class :

internal static class ErrorMessage
{
    public const string NoAccess = "Error accessing api - check internet connection/api address";
}

And you can reference that by using :

result = new TextResult(ErrorMessage.NoAccess);

Or, you can use a resource file.

Answer 5

Not that I disagree with @Win's answer, I believe putting Error and Hello consts which are logically related to IFeature in unrelated static class for the sake of avoiding duplication might not be appropriate approach. If the objective is to avoid duplication then I would like to achieve it in the following manner:

public abstract class Feature:IFeature
{
    public static readonly string Error = "Error accessing api...";
    public static readonly string Hello = "Hello ...{0}";

    protected IWebApi webApi;

    protected Feature(IWebApi webApi)
    {
        this.webApi = webApi;
    }
    public async Task execFeature()
    {
        var o = _execFeature();
        IResult result;
        if(o==null)
            result = new TextResult(Error);
        else 
            result = new TextResult( string.Format(Hello, o);
        result.display();
    }
    protected abstract object _execFeature();
}

So now I have achieved not only optimum minimization of code duplication, put Error and Hello where they are logically belong to. First and second Feature classes now can inherit from Feature class:

public class FirstFeature:Feature
{
    public FirstFeature(IWebApi webApi):base(webApi){}

    protected override object _execFeature ()
    {
        //your code for first Feature
        //return response if no error else return null
    }
}

public class SecondFeature:Feature
{
    public SecondFeature(IWebApi webApi):base(webApi){}

    protected override object _execFeature ()
    {
        //your code for second Feature
        //return class name[0] if no error else return null
    }
}

So that's what my design would be.

Answer 6

Apart from the fact that your feature is displaying messages, which I think it should not do, you clearly have a problem with localization if you put messages in your code. No matter where you hold it and how you access it, you don't want to recompile to have a different language. You may not even want to recompile to fix a stupid spelling mistake.

In your code, use error codes. This can for example be enums. Then in your front end application, have a translator service (.NET has a pretty nice feature called "resources" for this).

This way, you can have different resources for different front ends (like one for French. Or maybe one for children with easier language. Or one for technical people who can handle the real deal.)

Additionally, you can also have an automated front end that can react to codes instead of having to parse error messages and throwing a fit whenever someone corrects a spelling mistake or finds a friendlier term for something.