CODEWITHSUNDEEP
c#abstract-class
killerbunnyattack
killerbunnyattack

Reputation: 364

To abstract, or not to abstract

thanks in advance for reading this. I don’t fully understand how/when to use abstracts so I am trying to think about it each project I work on to see if it will all click some day Smile | :)

Also, the mix of accessibility levels (private, protected, internal) with keywords static, abstract, and override tend to leave me a little confused. How do I define this method/property/class....

It's not all a big mystery to me but some projects have me coding in circles when dealing with these topics.

With that said,

I have an application that reads an XML document and outputs text and image files. I’m also storing all of the information in a database. I have it working nicely.

The XML has a standard implementation with required fields and is used by multiple organizations to submit data to my app. All organizations should use (at least) the required nodes/elements that are outlined in the XML implementation guide.

So, I want to have a default data object type to be able to derive a specific organization’s data type for required elements. (If this object is going to be used, these are the fields that must be implemented).

If the org. just uses the default requirements, I can use the default object. If they use additional (optional) fields, I’ll have to create a new type inheriting the default type.

My first thought was to use and abstract class that had protected properties for my bare minimum requirements:

public abstract partial class AbstractDataObject
{
   protected string DataObjectName;
   protected DateTime? DataObjectDate;
   etc...
}

Then, if the organization just uses the required elements of the node and no optional elements, I can use a “default” object.

internal partial class DefaultDataObject : AbstractDataObject
{
   public new string DataObjectName { get; set; }
   public new DateTime? DataObjectDate { get; set; }
   etc...
}

But, if an organization uses optional fields of the required node, I can use a derived organization data object.

internal sealed partial class OranizationDataObject : AbstractDataObject
{
   public new string DataObjectName { get; set; }
   public new DateTime? DataObjectDate { get; set; }
   etc...

   //Optional fields used by this organization
   public string DataObjectCode { get; set; }
   etc...

}

Do I need the abstract class? It seems to me I can just have a DefaultDataObject (something like):

internal partial class DefaultDataObject
{
   public virtual string DataObjectName { get; set; }
   public virtual DateTime? DataObjectDate { get; set; }
   etc...
}

And then:

internal sealed partial class OranizationDataObject : DefaultDataObject
{
   public override string DataObjectName { get; set; }
   public override DateTime? DataObjectDate { get; set; }
   etc...

   //Optional fields used by this organization
   public string DataObjectCode { get; set; }
   etc...

}

I’m just really trying to understand how to define these objects so I can reuse them per organization. Both ways seem to work, but I am hoping to understand how to define them properly.

Getting the XML into above objects:

public DefaultDataObject ExtractXmlData(XContainer root)
    {
        var myObject = (from t in root.
        Elements("ElementA").Elements("ElementB")
              select new DefaultDataObject()
              {
        DataObjectName = (String)t.Element("ChildElement1"),
        DataObjectDate = 
                      Program.TryParseDateTime((String)
                      t.Elements("ChildElement2")
                      .ElementAtOrDefault(0)
        ),
        etc....

OR

public OranizationDataObject ExtractXmlData(XContainer root)
    {
        var myObject = (from t in root.
        Elements("ElementA").Elements("ElementB")
            select new OranizationDataObject()
              {
    DataObjectName = (String)t.Element("ChildElement1"),
    DataObjectDate = Program.TryParseDateTime(
             (String)t.Elements("ChildElement2")
             .ElementAtOrDefault(0)),
    DataObjectCode = (String)t.Element("ChildElement3"),

etc....

Again, thanks for reading. Don't forget to tip your wait staff....

Joe

Upvotes: 4

Views: 706

Answers (4)

radarbob
radarbob

Reputation: 5101

Everything I need to know I learned from Sesame Street
Scrub your class design hard to make sure you've identified everything that is the same and different. Play computer, so to speak, with your classes and see how they do the same, different, or the same thing but in different ways.

What is the same, different, same but differently will likely change as you play computer.

Think in general terms of the two pillars of OO Classes. Polymorphism and Inheritance
As you do the above that is. Not so much in terms of C# implementation per se.

How things clump into same vs. different will help drive implementation
And it's all relative.

  • More of same default behavior? Perhaps a concrete base class instead of abstract.
  • More of same thing, but differently? Perhaps an abstract class instead of concrete base class.
  • A default way of doing x? Perhaps a virtual method.
  • Everyone does the same thing, but no two the same way? A delegate perhaps.

Implementation Suggestions

  • Make methods and fields protected as a default. Private does not get inherited. Designs change, stay flexible. If something just has to be private, fine.

  • virtual means you can change implementation in a sub class. It does not mean you must.

  • Folks seem to under-utilize delegates. They're super for polymorphic methods.

  • There is nothing wrong with public fields. What's the practical difference between a public field and a public auto-implemented property? Nothing. They both directly return (or set) the underlying value. So what's the point of even bothering with properties? If you want to publicly expose an underlying value differently than it's "natural" state. For example, returning a number in a specific format. And of course you can have different properties for the same field.

  • A Property can have a get without a set. Or vice versa. Also get and set can have different access levels. Often you'll see this as a public get and a protected (or private) set.

Upvotes: 3

vgru
vgru

Reputation: 51204

  1. First of all, your base class doesn't need to be abstract if it's a plain DTO class. If you don't have any functionality that needs to be implemented differently by derived classes, you can simply make it a plain base class which will hold common properties.

  2. Next, there is no point in declaring properties in the base class (abstract in your case), if you are going to hide them (using the new keyword). You first code snippet of DefaultDataObject unnecessarily creates a bunch of new properties with the same name. Remove them completely - they are already defined in the base class.

    [Edit] I didn't notice this initially, and @svick warned me, that your base class actually contained fields instead of properties, which makes me wonder why you needed to add the new keyword at all. I went over your code quickly and saw them as properties. In any case, you should never expose public fields - at least change them to auto-implemented properties by adding the { get; set; } block.

    In other words, this would simply work:

    // this doesn't need to be abstract.
// just put all the common stuff inside.
public class BaseDO
{
    // as svick pointed out, these should also be properties.
    // you should *never* expose public fields in your classes.

    public string Name { get; set; }
    public DateTime? Date { get; set; }
}

// don't use the new keyword to hide stuff.
// in most cases, you won't need that's behavior
public class DerivedDO : BaseDO
{
    // no need to repeat those properties from above,
    // only add **different ones**
    public string Code { get; set; }
}

  3. As a side note, but nevertheless important IMHO, you should simplify naming (and make it more clearer what your code does). There is no need to repeat "DataObject" in every property name, for example. But since your code is probably only a simplified version, it doesn't matter.

  4. Lastly, have you heard of XmlSerializer? You don't need to traverse the XML elements manually. It is enough to call XmlSerializer to both serialize and deserialize your data.

Upvotes: 4

Olivier Jacot-Descombes
Olivier Jacot-Descombes

Reputation: 112279

An abstract class can already implement things that can be inherited

public abstract class DataObjectBase
{
    public string DataObjectName { get; set; }
    public DateTime? DataObjectDate { get; set; }
}

A concrete class can add new properties and methods

public class DerivedDataObject : DataObjectBase 
{
    public int NewProperty { get; set; }
}

The properties DataObjectName and DataObjectDate are already available in the new class, because they are automatically inherited from the base class.

If the abstract class defined an abstract member, however, you would have to implement it in the derived class.

Say the base class defines

public abstract void SomeMethod(string name);

The the derived class has to do this

public override void SomeMethod(string name)
{
    ...
}

If your base class does not have abstract members, it does not need to be abstract and can play the role of your default data object directly.

The keyword 'partial` is not needed here. It is only useful if you want to split one class into several pieces over several files.

The keyword new is wrong here. It is used to shadow an inherited member. This means that the inherited member will be hidden "behind" the new declaration. What you need, is to override. This does not hide a member, but provide an alternative implementation of the same member in the derived class.

Upvotes: 1

svick
svick

Reputation: 244757

It depends on what the derived types will want to do. If they are going to use the default implementation and only expand on it somehow, then having the default class as the non-abstract base class is fine.

On the other hand, if they are most likely going to re-implement the functionality, you should have an abstract base class (or an interface) and a separate default class.

If you for some reason don't know which one is it, you can let the inheritors choose by having an abstract base class and leaving the default class unsealed.

Also, looking at your code, it seems you misunderstand what the various keywords do. Most of the time, you do not want to use new like this. What it does is to define another member with the same name, unrelated to the original one. Also, there's no reason to override something if you don't want to change it. So, if you expect that the derived classes won't have to reimplement the properties, you don't have to make them virtual at all.

Upvotes: 2

Related Questions