Factory pattern to build multiple models

Question

I have used the factory pattern for one of my projects. I want to know if this is a good design or if my code can be improved to make it as fast as possible.

I am not an expert in C#. This is what I came up with after 2 days of research, with lots of rewrites. What I want to ask is:
Is this the right approach? Can I improve it to be faster and safer?

Source Problem: There is incoming data which includes JSON data. Data structure is like:

char.base {"level": 20, "name": "somename"}

What I call first char.base part is the module. There are 12 modules like this and each of them have different properties.

// ./Models/Model.cs
namespace Example.Models
{
 public abstract class Model
 {
 }
}

Those 12 models have their own classes which uses this Model class as their base. Example Model:

// ./Models/Base.cs
namespace Example.Models
{
 public class Base : Model
 {
 public int Level { get; internal set; }
 public string? Name { get; internal set; }
 }
}

Factory class:

namespace Example
{
 internal abstract class Factory
 {
 public Type Type { get => typeof(Factory); }
 internal abstract void Deserialize(string json);
 }
 internal class Factory<T> : Factory where T : Model
 {
 public new Type Type { get => typeof(T); }
 private JsonSerializerOptions _options = new() { PropertyNameCaseInsensitive = true };
 private Root _root { get; set; }
 internal Factory(Root root)
 {
 _root = root;
 }
 internal override void Deserialize(string json)
 {
 T? obj = JsonSerializer.Deserialize<T>(json, _options);
 if (obj == null)
 throw new ArgumentNullException($"{typeof(T)} cannot be deserialized with the given data: {json}");
 _root.AddOrUpdate(obj);
 }
 }
}

Finally Root class that gets the incoming data and sends to the right factory:

namespace Example
{
 public class Root
 {
 private readonly Dictionary<string, Factory> ModelFactory;
 private readonly ConcurrentDictionary<Type, Model> Models = new();
 public Root()
 {
 ModelFactory = new()
 {
 { "char.base", new Factory<Base>(this) },
 // 11 more like this
 };
 }
 public void Parse(string text)
 {
 int i = text.IndexOf(' ');
 if (i < 0)
 return;
 // text: char.base {...}
 // text: [module] [JSON]
 string module = text[..i].ToLower();
 string json = text[(i + 1)..];
 if (!ModelFactory.ContainsKey(module))
 return;
 ModelFactory[module].Deserialize(json);
 }
 public T? Get<T>() where T : Model, new()
 {
 return (T)Models.GetOrAdd(typeof(T), _ => new T());
 }
 internal void AddOrUpdate<T>(T value) where T : Model
 {
 Models.AddOrUpdate(typeof(T), value, (key, oldValue) => value);
 }
 }
}

The codes that needs these models will get it like this:

var base = root.Get<Base>();

Edit

Here is some sample incoming data:

char.base { "name": "Valour", "class": "Warrior", "subclass": "Soldier", "race": "Elf", "clan": "wolf", "pretitle": "Testing ", "perlevel": 1000, "tier": 1, "remorts": 7 }

char.vitals { "hp": 100000, "mana": 90000, "moves": 41599 }

comm.tick { }

There is nothing going on with the Model classes. Just some getter properties according to these JSON data.

• Valour: Maybe I abstract generic model class would be more suitable?
• Valour: Or maybe simple switch statement? But I will have to rewrite same code for every model with against with DRY coding?
• Peter Csala: Welcome to CodeReview! Could you please share with us two or three sample inputs and related Model derived classes? It would give us more context why did you choose Factory method.
• iSR5: There are 12 modules like this and each of them have different properties. can you give us two different samples of the Json data
• Valour: I added some sample data @PeterCsala

Answer 1

I think you have over-engineered the problem. You don't need neither the Factory nor the Factory<T> classes.

public class Root
{
 private readonly Dictionary<string, Type> ModelMapping;
 private readonly ConcurrentDictionary<Type, Model> Models = new();
 private readonly JsonSerializerOptions Options = new() { PropertyNameCaseInsensitive = true };
 public Root()
 {
 ModelMapping = new()
 {
 { "char.base", typeof(Base) },
 // 11 more like this
 };
 }
 public void Parse(string text)
 {
 int separatorIdx = text.IndexOf(' ');
 if (separatorIdx < 0) return;
 string module = text[..separatorIdx].ToLower();
 if (!ModelMapping.ContainsKey(module)) return;
 string json = text[(separatorIdx + 1)..];
 var parsed = Deserialize(json, ModelMapping[module]);
 Models.AddOrUpdate(ModelMapping[module], parsed, (key, oldValue) => parsed);
 }
 private Model Deserialize(string json, Type parseTo)
 => JsonSerializer.Deserialize(json, parseTo, Options) as Model
 ?? throw new ArgumentNullException($"{parseTo} cannot be deserialized with the given data: {json}");
 public T? Get<T>() where T : Model, new()
 => (T)Models.GetOrAdd(typeof(T), _ => new T());
}

• I've replaced your ModelFactory to ModelMapping (maps string to Type)
• I've moved the Deserialize method from the Factory to here
  • I've also changed the return type from void to Model
  • Now the Parse can call the AddOrUpdate method
• I've made the Deserialize work on Type rather than expecting a T type parameter

---

I've used to following code for testing:

var @base = new Base { Name = "test", Level = 20 };
var source = @"char.base " + JsonSerializer.Serialize(@base);
var root = new Root();
root.Parse(source);
var newBase = root.Get<Base>();
Console.WriteLine(newBase.Level == @base.Level);

• Valour: Thanks. It seems I really over-engineered the problem. I always forget the easy method is the best method most of the time!
• iSR5: @Valour In addition of @Peter-Csala answer, It would be more feasible if you just make use of JsonSerializerOptions and add custom JsonConverter to work with these incoming Json. find out more about it here docs.microsoft.com/en-us/dotnet/standard/serialization/…
• Valour: Can I use records instead of class in models with this code?
• Peter Csala: @Valour yes you can.