CODEWITHSUNDEEP
c#.netoop
Dav.id
Dav.id

Reputation: 2797

c# practices for correct object oriented techniques

I have some c# code that has been working well for a while now.. and I have to say, whilst I understand the basics of OO principles, there is obviously more than one way to skin a cat (although I hate that phrase!).

So, I have a base abstract class that is acting as a basic data service class as follows (much simplified just for ease of reading):

public abstract class dataservice
{
    public enum OutputType : int { XmlTOJson = 0, Xml = 1, Json=2 }

    protected object SomeDBcall(string StoredProcedure)
    {
        // Just assume we are using SQLclient/DB access..
        object SomeReturnObjValue = db.ExecuteScalar(cmd);
        return SomeReturnObjValue;
    {
}

.. so basically I might have a few basic DB retrieve/update/delete calls in the abstract class.. mainly as there are the basis of any DB operation I have in my app.

So now we have a class that implements the base class, say in my case a customer class:

public class Customer : dataservice
{
    Public String CustomerDoSomething(string SomeDataEtc)
    {
        // Ok, so again for simplicity sake, we are going to use the base class to 
        // call a DB retrieve 
        object ReturningObj = SomeDBcall("my stored procedure");
        return ReturningObj.ToString();
    }
}

So I guess my question is this: Is the above method "ok" to use? considering a virtual method could be over-ridden if required, however in this case I only want the base class to use those methods which are protected as the means to call the DB operations.

Any clarity/guidance very appreciated!

Upvotes: 3

Views: 836

Answers (5)

Larry Hector
Larry Hector

Reputation: 183

You might want to look to the Template pattern to define the interface in the base (abstract or not) class with defined protected virtual hooks that can be overridden in the concrete subclasses. As mentioned by another poster, if you just intend to add DB services to each of your domain areas you might look to encapsulate the basic database service methods into a helper class rather than deriving from the database service.

Thanks @jgauffin for questioning my LSP violation statement. It was not correct and has been removed. There are lots of cases where extending the public interface of the base class by subclasses is warranted. Of course, by doing that one needs to be careful that you have an instance of a Y and not an X or a Z when performing a Y-specific operation A(), assuming that both Y and Z derive from X where Y adds the new public method A() and Z does not.

An example of the Template pattern in the OP's context would allow better encapsulation of custom functionality within subclasses without extending the public interface. However, this only works if there is not external influence exerted on the subclass instance, such as the OP's SomeDataEtc parameter. This works best when the instance is immutable.

public abstract class DataService
{
    protected object myWidget = new Widget();
    public object SomeDataBaseCall(string storedProcedure)
    {

        DoSomeCustomThing();

        //do db stuff
        object SomeReturnObjValue = db.ExecuteScalar(storedProcedure);
        return SomeReturnObjValue;
    }

    protected void DoSomeCustomThing() {}
}


public class Customer : DataService
{
    override protected void DoSomeCustomThing()
    {
        // do your custom thing here
    }
}

Additionally, in the OP's example, it would seem prudent to use delegation within the derived class's new public method to call the base class's SomeDBCall method to execute the stored procedure. If you are redundantly coding the db access methods then there is no benefit to the proposed inheritance.

As was also mentioned elsewhere, you might be better off altogether by using composition rather than inheritance for the data service functionality.

Upvotes: 1

umlcat
umlcat

Reputation: 4143

Overview

Many times, your "development itself will guide you".

Practical answer.

(1) You define a base class "dataservice", and from that class, several other classes will be based upon. You marked as "abstract", thats good. It's not mean to have variables by itself.

Some developers won't mark that class as "abstract", its not obligatory, but, its a not a bad idea, but, its a "good practice", to marked "abstract".

And, other methods will be added, used by the subclasses, maybe overriden, maybe not.

For know, those methods are protected, and anot mean to be used outside the object, but, by other methods. That's ok.

Maybe, later, a method may be required to be used outside the class, and may have to change to public.

(2) You add a subclass "Customer" that is a descendant from "DataService" You add a method that has to be used outside the class, and marked as "public", good.

It's only meant to be used by this class, not the parent class. So, no "virtual" or "override" required. Good.

(3) Your example its very simple. Most things you did, seems fine to me.

Eventually, when you add more code, things may change, example a method in the base class that was private may become public, or you may "rename" or "refactor" a method, like "dosomething", and found out that its better to be in the base class, or maybe not.

Summary

There are other answers, that mention, rules, or concepts. Seems to me that they are OK, but, skip the fact that you are learning to use O.O.P. better. Some people just try to "eat the cake in one wingle big bite", and that's not a good idea.

P.D. "can ur skin ur rabbit", sounds better to me.

Cheers.

Upvotes: 1

jgauffin
jgauffin

Reputation: 101140

No. Guess your following data access object pattern (DAO). Either way your Customer is not your data access class. It uses a class for data access. What I mean is that your DAO should favor composition over inheritance.

Something like:

public class Customer : IDataAccessObject
{
    public Customer()
    {
        _dataAccess = new DataAccess();
    }

    public string CustomerDoSomething(string SomeDataEtc)
    {
        object ReturningObj = _dataAccess.SomeDBcall("my stored procedure");
        return ReturningObj.ToString();
    }
}

Why? Your objects get's a single responsibility which means that it's easier to extend and refactor them.

You can read up about SOLID which is some fundamental programming principles.

Since you are a .NET developer I also recommend that you embrace the naming guidelines.

Upvotes: 0

Mare Infinitus
Mare Infinitus

Reputation: 8162

Perhaps you can let your abstract class be concrete and use it as some kind of helper class which handles the database related stuff you need. As far as the example code shows, there is no need to have multiple database accessing classes, just different parameters.

Upvotes: 1

Ed Swangren
Ed Swangren

Reputation: 124642

Sure, it's "ok", though I see no reason for the base class to be abstract. abstract classes are great for implementing some common logic and leaving the rest up to derived classes to implement. However, you have no abstract/virtual methods, so I don't see the point here.

Upvotes: 1

Related Questions