C# class design principle

Question

Is it good practice to have class (name is Catalog) without methods and have another Manager class (CatalogManager) with methods for manipulation of the Catalog objects?

Or, class should have own methods. Thanks for your time.

namespace ESF.Configurator.CatalogManagement
{
    public class Catalog
    {
        public Catalog()
        { 
        }

        private int _ID;
        private string _Name;
        private string _Synonym;
        private string _Description;

        public int ID
        {
            get { return _ID; }
            set { _ID = value; }
        }

        public string Name
        {
            get { return _Name; }
            set
            {
                _Name = value;
            }
        }

        public string Synonym
        {
            get { return _Synonym; }
            set { _Synonym = value; IsDirty = true; }
        }

        public string Description
        {
            get { return _Description; }
            set { _Description = value; }
        }
    }


    public class CatalogManager
    {
        public CatalogManager()
        {
        }


        public Dictionary<int, Catalog> Catalogs;

        public Catalog CreateNewCatalog()
        {
        }

        public void SaveCatalogToDB(Catalog catalog)
        {
        }


        public void CreateCatalogInDB(Catalog catalog)
        {
        }

        public void UpdateCatalogInDB(Catalog catalog)
        {
        }

        public void DeleteCatalogFromDB(Catalog catalog)
        {
        }

        public string GenerateCatalogXML(Catalog catalog)
        {
        }

        public void PopulateCatalogList()
        {
        }

        public Catalog ConvertDataRowToCatalog(DataRow dataRow)
        {
        }

        public DataSet FetchAllCatalogs(int ConfigID)
        {
        }
    }
}

Answer 1

You would mostly prefer your classes to have high cohesion, and that's why it is better to have all methods of a module lie within itself. But there are some caveats I am going to mention, in which delegating the behavior to another class is an advantage. To start with, look at the following definition (from Wikipedia).

In computer programming, cohesion refers to the degree to which the elements of a module belong together.

However, to solve some programming issues*, you might sacrifice your cohesiveness to decouple your modules or to let them be extensible (*OCP principle). We achieve this through delegation.

*A well known solution for a particular issue / problem, a.k.a a design pattern, is the Strategy pattern. This pattern delegates its behavior (also called a behavioral pattern) from a class to another class letting this behavior / algorithm to be changed at run-time. This is an example of separating the methods from a class.

*In OOP, modules should be open for extension and closed for modification. This is the Open/closed principle (=OCP). You could use inheritance instead of the Strategy pattern. But doing so you are coupling the behavior to the module. This is not necessarily bad, but to let the algorithms vary independently, and to couple your clients to an interface (abstraction) instead of a realization (implementation), use the Strategy pattern. This way your clients won't experience any upheaval associated with a change.

---

Regarding your examples, if you have a good reason to separate concerns (SoC) with delegating your CROUD behavior to the "CatalogManager" - go for it. You should ask yourslef: "Does it solve me any existing problem?".

Answer 2

Here's a sample you might want to consider.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace TestOnly
{
    public class TestClass
    {

        public void Test()
        {
            Catalog catalog = new Catalog();

            //Perform save
            SaveCatalogAction save = new SaveCatalogAction();
            save.Catalog = catalog;
            CatalogManager.Do(save);

            //Perform delete
            DeleteCatalogAction delete = new DeleteCatalogAction();
            delete.Catalog = catalog;
            CatalogManager.Do(delete);

            //Perform update
            UpdateCatalogAction update = new UpdateCatalogAction();
            update.Catalog = catalog;
            CatalogManager.Do(update);
        }

    }

    public class Catalog
    {
        public int Id { get; set; }
        public int Name { get; set; }
    }

    public class CatalogManager
    {
        static public void Do(CatalogAction catalogAction)
        {
            catalogAction.Perform();
        }
    }

    public abstract class CatalogAction
    {
        //Note we can move this CatalogList to somewhere appropriate (like in CatalogManager) it's here for sake of simplicity
        public Dictionary<int, Catalog> CatalogList = new Dictionary<int, Catalog>();
        public Catalog Catalog { get; set; }

        public abstract void Perform(); //Or you can put the Catalog as parameter
    }

    public class SaveCatalogAction : CatalogAction
    {

        public override void Perform()
        {
            //Do saving with catalog
            if (base.Catalog != null)
            {
                //Save
                base.CatalogList.Add(base.Catalog.Id, base.Catalog);
            }
        }
    }

    public class DeleteCatalogAction : CatalogAction
    {

        public override void Perform()
        {
            //Do deleting
            if (base.Catalog != null)
            {
                //Delete (note no checking just to make it simple)
                base.CatalogList.Remove(base.Catalog.Id);
            }
        }
    }

    public class UpdateCatalogAction : CatalogAction
    {

        public override void Perform()
        {
            //Do updating
            if (base.Catalog != null)
            {
                //Update  (note no checking just to make it simple)
                base.CatalogList[base.Catalog.Id] = base.Catalog;
            }
        }
    }
}

Some points:

• Catalog object (which could be persistent), I think would be better if all are properties only. Like the one you have.
• Have a Manager, like CatalogManager, to perform the action as described above.
• Have each Catalog action to be on one class, to avoid bloated codes. Remember this when designing class: https://en.wikipedia.org/wiki/Single_responsibility_principle

Hope this pattern would help!

Answer 3

I generally have any functions related to database storage in a separate class (maybe something like CatalogRepository) which is responsible for any database transactions. Your Catalog class should not have the storage logic mixed in its implementation. It is better practice to abstract this away from your class, as for example you may one day wish to change the database you are using (say from SQL Server to SQLite). Having the database transaction logic abstracted from your class implementation means your class doesn't need to know (or care) about whether you are writing to a text file or storing in a database.

Also, functions like "CreateNewCatalog" might be better situated in a Factory (say CatalogFactory). It would be responsible for building a Catalog and returning it to you, as you would already need to have an instance of a Catalog to call "CreateNewCatalog" with the code above.

So, in answer to your question, any functions related to persisting and creating the class in question should not be included within that class, and rather be the responsibility of another class (or classes), as you have done with CatalogManager.

Answer 4

It could be ok, so you separate model layer (Catalog) from controller layer (ControllerManager). Many OS use MVC(model-view-controller) architecture.

More info here