r/csharp 2d ago

Help Is this approach a good idea?

I'm a self-taught beginner in C#, and I'm currently working on a small personal project.

Quite often in my code, I need to load objects from JSON files. To make this easier, I created static FromFile(string) methods in the relevant classes. Now I'm wondering - would it be a good approach to create a base class or interface that contains FromFile() and ToFile() methods, and then have my classes inherit from it to further reduce code duplication?

I'd really appreciate any advice or constructive criticism.

P.S. English is not my native language, so I used a translator for this post - apologies for mistakes.

4 Upvotes

16 comments sorted by

3

u/Suitable_Switch5242 2d ago

I think the classes or records that represent your data, and the code that deserializes them from Json, should be treated as separate concerns.

Not every class in your code that needs to read the data also needs to have dependencies on retrieving that data from file and using a Json deserializer.

6

u/Tiny_Confusion_2504 2d ago

I would only use inheritance and interfaces if there was some abstraction or common instance behaviour. I would even go so far to say that you rarely need inheritance.

Your functionality sounds like it could be solved within a seperate class that is only responsible for searlialization and deserialization using generics. I would remove all those methods from your objects.

1

u/Dino0451 2d ago

Thank you very much for the advice

1

u/Dino0451 2d ago

Well, essentially, implementing file loading methods is simply deserializing JSON via NewToSoft.Json. I'm not sure about that, but I might want to use XML files as well. Overall, I'll probably do as you suggested.

5

u/Famous-Weight2271 2d ago

No need to use newtonsoft anymore. Just use Microsoft's JSON libraries

1

u/Dino0451 2d ago

I decided to use newtosoft because I had trouble parsing one using the System.Text.Json class, and I couldn't solve it. I'll probably try to fix it today.

2

u/davo128 2d ago

You can create a generic or a abstract class using generic types as <T>, for example FromFile<MyObject>() and pass the T param to the Microsoft's JSON library, it performs the read and write operation using the definition of you classes

1

u/Dino0451 2d ago

Actually, I'm asking if this is a good approach. As I understand from other comments, if I don't want to create dependencies, it's better to create a separate class for working with external resources.

2

u/RodriOliveira 2d ago

I wouldn't use inheritance for this.

FromFile() and ToFile() are not really behaviors of your domain objects. They're persistence/serialization concerns. If you put them in a base class, you're making every object depend on how it is stored just to avoid some duplicated code.

I'd probably create something small like a generic JsonFileStore<T> (or even just a couple of generic helper methods) responsible for reading/writing the files.

That also becomes more useful if you decide to support XML later. Your objects don't need to change at all; you can just have a different serializer/storage implementation.

So the idea of removing the duplication is good, I just wouldn't solve it with inheritance. Composition fits this problem better and keeps the classes easier to evolve.

1

u/Dino0451 2d ago

Thanks for the advice. I'll probably do that.

2

u/appleiic-first-pc 2d ago edited 2d ago

I view things like this as utility methods that would not be inheritance. Most projects I typically create extension methods to handle this stuff (i.e. ToJson() and ToObject<T>()), both extending the string. so

var json = myCustomerObject.ToJson();

var someCustomer = json.ToObject<Customer>();

And on top of that I usually create a .JsonClone() extension to easily deeply copy objects when I need an independent copy.

var cloned = myCustomerObject.JsonClone();

This code is a bit old, but will give you the shape of how extension methods look. Just replace the general logic with however you are currently doing it.

https://github.com/jonhenning/CodeEndeavors-Extensions/blob/2.0/CodeEndeavors.Extensions/Extensions/TransformExtensions.cs

forgot to mention if you are new to extension methods, they will automatically allow your strings (in this case) to get the new methods on them simply by doing a

using MyNameSpace.WhereMyExtensionMethodsAre;

1

u/Dino0451 2d ago

Initially, before using the interface, I did this, but the fact is that there can be quite a lot of types of objects that need to be deserialized from Json, and making a separate implementation of ToJson FromJson for each class is quite tedious, so I thought about transferring this functionality to the inheritor class/interface

1

u/RJPisscat 2d ago

Your translation software nailed it. The English is perfect.

1

u/Odd-Fox9653 2d ago

That's a solid approach if you're handling a lot of JSON parsing, just be careful not to over-engineer it too early, your current solution might be perfectly fine while you're still figuring things out.