Making a Custom class less specialized but just as understandable?

Question

In my C# program, I have a class defined as follows:

public class DbResult
{
    public bool bSuccess { get; private set; }
    public String Message { get; private set; }

    private DbResult(bool success, string message)
    {
        this.bSuccess = success;
        this.Message = message;
    }

    public static DbResult Failed(string message)
    {
        return new DbResult(false, message);
    }

    public static DbResult Success(string message)
    {
        return new DbResult(true, message);
    }
}

This is used in my DataAccess class, which has several methods whose return type is DbResult. They use LINQ and the Entity Framework. UPDATE Here is the entire DataAccess as of right now:

public class DataAccess
{
    private readonly TestDatabaseEntities _MyDBEntities;

    public DataAccess(TestDatabaseEntities entities)
    {
        _MyDBEntities = entities;
    }


    public DbResult DatabaseExists()
    {
        DbResult MyResult;

        if (_MyDBEntities.Database.Exists())
            MyResult = DbResult.Success("Database Found");
        else
            MyResult = DbResult.Failed("Database Not Found");

        return MyResult;
    }

    // ============================
    // CRUD FUNCTIONS for MAN TABLE
    // ============================

    public DbResult Create(Man M)
    {
        DbResult DbRes;

        try
        {
            _MyDBEntities.Men.Add(new Man { ManID = M.ManID, Name = M.Name });
            DbRes = DbResult.Success("Record created");
        }
        catch (Exception e)
        {
            DbRes = DbResult.Failed(e.ToString());   
        }            
        return DbRes;
    }

    public DbResult Update(IQueryable<Man> myQuery, Man man)
    {
        DbResult DbRes;

        try
        {
            foreach (Man M in myQuery)
            {
                M.Name = man.Name;
            }
            DbRes = DbResult.Success("Record updated");
        }
        catch (Exception e)
        {
            DbRes = DbResult.Failed(e.ToString());   
        }

        return DbRes;
    }

    public DbResult Delete(IQueryable myQuery)
    {
        DbResult DbRes;

        try
        {
            foreach (Man M in myQuery)
            {
                _MyDBEntities.Men.Remove(M);
            }
            DbRes = DbResult.Success("Record deleted");
        }
        catch (Exception e)
        {
            DbRes = DbResult.Failed(e.ToString());                
        }

        return DbRes;
    }

    public DbResult Read(IQueryable myQuery, out string[,] Records)
    {
        DbResult DbRes;
        Records = null;
        try
        {
            List<Man> men = myQuery.OfType<Man>().ToList();
            Records = new string[men.Count, 2];
            for (int i = 0; i < men.Count; i++)
            {
                Records[i, 0] = men[i].ManID.ToString();
                Records[i, 1] = men[i].Name;
            }
            DbRes = DbResult.Success("Read Success");
        }
        catch (Exception e)
        {
            DbRes = DbResult.Failed(e.ToString());
        }

        return DbRes;
    }

    // ============================
    // SAVECHANGES FUNCTION
    // ============================
    public DbResult SaveChanges()
    {
        DbResult DbRes;
        try
        {
            _MyDBEntities.SaveChanges();
            DbRes = DbResult.Success("Saved successfully");
        }
        catch (Exception e)
        {
            DbRes = DbResult.Failed(e.ToString());
        }

        return DbRes;
    }
}

I also have a class called ServiceManager, which checks to see if the SQL Server (SQLEXPRESS) service is running on the local machine.

public class ServiceManager
{
    public DbResult SQLRunning()
    {
        DbResult MyResult;
        ServiceController sc = new ServiceController("SQL Server (SQLEXPRESS)");

        if (sc.Status == ServiceControllerStatus.Running)
            MyResult = DbResult.Success("SQL Server is running");
        else
            MyResult = DbResult.Failed("SQL Server is NOT running.");

        return MyResult;
    }
}

As you may have noticed in the code directly above, I use the DbResult class as a return type to carry the status message and bool back to the main program. I think this does not make sense since DbResult is a class created for returning results from trying to perform operations on the Database, not the SQL Server checkup. I could create a new class called ServiceResult, which does the exact same thing as DbResult, but that would be duplicating code. see example below:

public class ServiceResult
{
    public bool bRunning { get; private set; }
    public String Message { get; private set; }

    private ServiceResult(bool bRunning, string message)
    {
        this.bRunning = success;
        this.Message = message;
    }

    public static ServiceResult Running(string message)
    {
        return new ServiceResult(false, message);
    }

    public static ServiceResult Stopped(string message)
    {
        return new ServiceResult(true, message);
    }
}

You see how the above class is very similar to the DbResult class and works relatively the same way? This is how I might make a class for Services, but is there any way I can make a class that does both DbResults and ServiceResults without becoming more ambiguous like the following class example below? or, is it ok to be ambiguous like the result I put below?

public class Result
{
    public bool bYesNo { get; private set; }
    public String Message { get; private set; }

    private Result(bool YesNo, string message)
    {
        this.bYesNo = YesNo;
        this.Message = message;
    }

    public static Result No(string message)
    {
        return new Result(false, message);
    }

    public static Result Yes(string message)
    {
        return new Result(true, message);
    }
}

UPDATED

The methods that use my DataAccess class looks like this

    static private void DoCreate()
    {
        int myID;
        bool bIsValidID;
        var dbEntities = new TestDatabaseEntities();
        string sNewName;

        DataAccess MyDA = new DataAccess(dbEntities);
        DbResult CreationResult, SaveResult;

        do
        {
            bIsValidID = _MyUI.GetValidInput<int>("Enter ID: ", int.TryParse, out myID);
        }
        while (!bIsValidID);

        sNewName = _MyUI.GetInput<string>("Enter Name:", x => x.Trim());

        CreationResult = MyDA.Create(new Man() {ManID = myID, Name = sNewName });
        _MyUI.DisplayMessage(CreationResult.Message);

        if (!CreationResult.bSuccess)
            return;

        SaveResult = MyDA.SaveChanges();
        _MyUI.DisplayMessage(SaveResult.Message);
    }
    static private void DoRead()
    {
        var dbEntities = new TestDatabaseEntities();
        DataAccess MyDA = new DataAccess(dbEntities);
        string [,] Records;
        DbResult ReadResult;

        var query = from person in dbEntities.Men
                    where true
                    select person;
        ReadResult = MyDA.Read(query, out Records);
        if (ReadResult.bSuccess)
        {
            _MyUI.DisplayRecords(Records);
        }
        if (!ReadResult.bSuccess)
            _MyUI.DisplayMessage(ReadResult.Message);
    }
    static private void DoUpdate()
    {
        int myID;
        var dbEntities = new TestDatabaseEntities();
        string sNewName = "";
        DataAccess MyDA = new DataAccess(dbEntities);
        DbResult UpdateResult, SaveResult;

        myID = _MyUI.GetInput<int>("Enter ID to update: ", int.Parse);
        sNewName = _MyUI.GetInput<string>("Enter new name: ", x => x.Trim());

        var query =
            from person in dbEntities.Men
            where person.ManID == myID
            select person;

        UpdateResult = MyDA.Update(query, new Man() { ManID = myID, Name = sNewName });
        _MyUI.DisplayMessage(UpdateResult.Message);

        if (!UpdateResult.bSuccess)
            return;

        SaveResult = MyDA.SaveChanges();
        _MyUI.DisplayMessage(SaveResult.Message);
    }
    static private void DoDelete() 
    {
        int myID;
        bool bValidInput;
        var dbEntities = new TestDatabaseEntities();
        DataAccess MyDA = new DataAccess(dbEntities);
        DbResult DeleteResult, SaveResult;

        do
        {
            bValidInput = _MyUI.GetValidInput<int>("Enter ID to delete: ", int.TryParse, out myID);
        } while (!bValidInput);

        var Query =
            from person in dbEntities.Men
            where person.ManID == myID
            select person;

        DeleteResult = MyDA.Delete(Query);
        _MyUI.DisplayMessage(DeleteResult.Message);

        if (!DeleteResult.bSuccess)
            return;

        SaveResult = MyDA.SaveChanges();
        _MyUI.DisplayMessage(SaveResult.Message);
    }

Answer 1

Can I try to convince you that you don't actually need such a Result class at first place?

How do you actually use the string Message value of this class? Do you show it to user? If yes, then what would you do when you need to make it multilingual? What I'm trying to say is that verbal interpretation of "success" or "failure" should actually be done on UI level, and Result class is not a UI. Also there is usually a single "success" case. You may say that you might want to differentiate several failure cases, but those are best described by a nice technology called exceptions.

So, what do we have beside Message? A boolean flag. That's what we probably want to return in case if we're interested whether something was successful or not... but wait. What usually happens when something goes wrong? Exception. And it would probably be more useful to let the business code process this exception, as the same exception may be treated differently in different situations.

Now let's go back to your code. You ServiceManager class doesn't have a state, so let's make it static:

public static class ServiceManager
{
    public static bool IsSqlServerRunning()
    {
        //TODO: What if service is not installed?
        ServiceController sc = new ServiceController("SQL Server (SQLEXPRESS)");
        return (sc.Status == ServiceControllerStatus.Running);
    }
}

Now it's much cleaner, and the caller of this method can decide what to output in case when SQL Server is not running...

About DB wrappers - I would suggest to avoid them at all. They represent abstractions over abstraction (Entity framework). By wrapping adds/deletes/selects in separate methods you loose lots of benefits provided by ORM frameworks like transactional nature of the DB interaction, without getting pretty much any benefit. It's actually much easier and cleaner to explicitly define context boundaries (in case of Web processing boundaries are already defined) and directly use the ORM to manipulate with the data.

Update based on updated question. There are several issues in the code that uses DataAccess class:

• All methods are static. It's not an issue for a small codebase and console app like yours, but don't overuse them.

• The following code is actually exactly the same as just dbEntities.Men

  from person in dbEntities.Men
  where true
  select person
  

• naming conventions (local variables should start with lower case
• don't convert Man objects into 2-dimensional array...

I'll show you how to simplify the code based on DoUpdate method, it includes most of the tasks done in other methods. No DataAccess class usage, cleaner code, better exception management.

static private void DoUpdate()
{
    int myID = _MyUI.GetInput<int>("Enter ID to update: ", int.Parse);
    string sNewName = _MyUI.GetInput<string>("Enter new name: ", x => x.Trim());

    try
    { 
        using (var dbEntities = new TestDatabaseEntities())
        {
            //TODO: If ManID is unique then we can use FirstOrDefault here.
            var allMatchingMen =
                from person in dbEntities.Men
                where person.ManID == myID
                select person;

            foreach(var man in allMatchingMen)
                man.Name = sNewName;

            dbEntities.SaveChanges();
            _MyUI.DisplayMessage("Record(s) updated");
        }
    }
    catch (OptimisticConcurrencyException ex)
    {
        //TODO: add auto-retry logic
        _MyUI.DisplayMessage("Someone updated the record, let's retry");
    }
    catch (Exception ex)
    {
        _MyUI.DisplayMessage("Something went wrong, could not update");
    }
}