I'm a beginner in desing pattern and I wanna to refactor some of my old codes for practice.

The code below is basically a JSON schema validator, I retrieve the schemas (RestoreJsonSchemas method) through a folder with .txt files, containing the standard JSON.

Then I use these saved schemas to validate the data I am getting in the IsValidJson() method.

My question is how would it be possible to apply the Open/Closed principle? Or should I think of another design pattern?

public class JsonSchemaValidator
{
    private const string connectionSchemaPath = "JsonSchemas\\schemaConnection.txt";

    private string connectionJsonDefault;

    public bool IsValidJson(string topic, string json)
    {
        try
        {
            if (topic != null && json != null)
            {
                JSchema schema;
                JObject jsonObject = JObject.Parse(json);

                switch (topic)
                {
                    case "connection":
                        schema = JSchema.Parse(connectionJsonDefault);
                        return jsonObject.IsValid(schema);

                        /// more cases...
                }
            }
        }
        catch(Exception ex)
        {
            // to be implemented
            return false;
        }

        return false;
    }

    public void RestoreJsonSchemas()
    {
        try
        {
            this.connectionJsonDefault = File.ReadAllText(connectionSchemaPath);

            /// more files to restore
        }
        catch (Exception ex)
        {
            // to be implemented
        }

        Console.WriteLine("Restored all JSON schemas.");
    }
}

CodePudding user response：

The most severe issue at the moment is that your switch is static, hardcoded into the class. This is against Open/Close as your class is closed to extensions.

To fix this, introduce an internal map from topics to schemas. Initialize this map with predefined values (like your connection) but add a public method for class clients so that they can add their own schemas.

Then, refactor your switch to iterate over this newly introduced map.

Roughly (haven't tested if this compiles at all)

public class JsonSchemaValidator
{
  private const string connectionSchemaPath = "JsonSchemas\\schemaConnection.txt";

  private Dictionary<string, string> schemaMap = new Dictionary<string, string>();

  public bool IsValidJson(string topic, string json)
  {
    try
    {
        if (topic != null && json != null)
        {
            JSchema schema;
            JObject jsonObject = JObject.Parse(json);

            foreach ( var schemaKey in schemaMap.Keys )
            {
                if ( schemaKey == topic )
                {
                    schema = JSchema.Parse(schemaMap[schemaKey]);
                    return jsonObject.IsValid(schema);
                }
            }
        }
        return false;
    }
    catch(Exception ex)
    {
        // to be implemented
        return false;
    }

    return false;
  }

  public void RestoreJsonSchemas()
  {
    try
    {
        this.schemaMap.Clear();

        // initialize defaults
        this.schemaMap.Add("connection", File.ReadAllText(connectionSchemaPath));

        // more defaults
    }
    catch (Exception ex)
    {
        // to be implemented
    }

    Console.WriteLine("Restored all JSON schemas.");
  }

  public void AddCustomSchema( string topic, string schema )
  {
     if ( schemaMap.ContainsKey( topic ) )
       schemaMap.Remove( topic );
     schemaMap.Add( topic, schema );
  }
}

This is open for extensions now. Defaults mapping are supported as long as the client calls RestoreJsonSchemas (I would even consider moving this into the constructor so that the client doesn't have to call it at all to have default mappings available). But, other mappings are also frelly supported thanks to the newly introduced AddCustomSchema.

Regardless of whether a mapping is a default one or a custom one, all mappings are taken into account in the validation method.