0

I have the following object model:

class CheckoutAd{
  int AdId;
  int ImpressionCap;
  int ClickCap;
  int ConversionCap;
  // ...
}

class SiteAd{
  int AdId;
  int ImpressionCap;
  int ClickCap;
  //...
}

As you see there is duplicate code. So i decided to put the duplicate code in a base class and extend the above classes from base class as follows:

class BaseAd{
   int AdId;
   int ImpressionCap;
   int ClickCap;
} 

But then I thought about the future which there will be a MobileAd class that this base class might not be a good candidate then I would have to change it.

Should I delegate these properties instead?

How would you do it?

Should i use strategy pattern ? how would i delegate this behavior?

DarthVader
  • 52,984
  • 76
  • 209
  • 300
  • By the way I think that you should share some more information about your intentions. It would make answering this question a little more easier. – Adam Arold Aug 09 '12 at 21:11
  • 1
    All I see is fields, not behaviour. – Oded Aug 09 '12 at 21:11
  • well that s the idea. If i put the common fields between these two classes. in the future i might have to break it. however if i convert them to beahviour in the future i wont have to break the hierarchy. – DarthVader Aug 09 '12 at 21:16
  • 2
    Do what makes sense now. No point in future proofing unless you **know** what is going to come. – Oded Aug 09 '12 at 21:17
  • What about the current base class would be inappropriate for a MobileAd class? Or, to put it another way, what behavior/properties *will* be common between the two current classes and MobileAd? Only you know the answer to that, since we currently have nowhere near enough information. – dlev Aug 09 '12 at 21:17
  • two of them will share cap behaviour, however, for example. no such notion in the emailads, but this is not yet implemented. – DarthVader Aug 09 '12 at 21:18

4 Answers4

3

I see no duplicate code, I see a couple of classes with members with the same names and types.

Is CheckoutAd a SiteAd? (yes inherit)
Does CheckOutAd have a SiteAd? (yes aggreate)
No to both, leave them the heck alone..
Tony Hopkinson
  • 20,172
  • 3
  • 31
  • 39
0

Since I cannot speculate on future classes, from what I see here, this is a perfect spot for either an interface or an abstract class.

An Absract class can have methods and properties filled in the need to be over-ridden.

Since you just have empty properties, an interface looks to be more appropriate.

Take a look at: C# interfaces - What's the point?

Community
  • 1
  • 1
Wesley
  • 5,381
  • 9
  • 42
  • 65
  • Then your question appears to be too vague. It seems as though it involves speculation, or possibly inadequate planning. – Wesley Aug 09 '12 at 21:16
  • which part wasnt clear? can u clarify what u didnt understand? i already mentioned base class. your answer repeats what i already know. – DarthVader Aug 09 '12 at 21:20
  • Building off of Oded's comments, I would use an interface to build the class as it makes sense now. If, in the future, you cannot fit MobileAd into the interface, then it shouldn't inherit. – Wesley Aug 09 '12 at 21:26
0

This is highly dependent on your application, your goals, and your time frame.

You need to implement whatever solution you feel will reduce complexity. If your objects are going to share behaviors and data, and inheriting them from a base class is going to make your application more understandable, more readable, and less prone to confusion, then by all means go forth with identifying all of the use cases of ads, build a base class, and inherit.

What you don't want to do is choose a design based upon some "possible" scenario that you haven't thought of yet. You're going to end up with something that doesn't make sense for the current state and unnecessarily contains a structure for something that may not even happen.

Identify all the possible types of ads that you can think of now. Identify if they share behavior, data, none, or both. If they share behavior, inherit from a base class that defines the behavior. If they share behavior and data, inherit from a base class that defines the behavior and data. If they share data, contain that data in instances of the classes.

Remember, your goal is to reduce complexity. Go with the simplest solution now and iterate on it when you run in to a problem. Don't design for problems that you speculate could occur.

Justin Skiles
  • 9,373
  • 6
  • 50
  • 61
0

I would not bother thinking any inheritance unless methods duplicated. You have common fields, so what? I fail to deduce what you meant precisely by ..., but lets say this is something meaningless. So, base class does not use generalized fields itself at all.

In my team we had some clashes over moving duplicated fields into base class. Leaving common fields in base class you can violate SRP. Speaking of the future: you may decide to provide functional getter/setter for a certain field to confer it some side effect. You can not control access completely, because it is visible through base class interface.

Fields with the same name in two classes should have distinct behavior associated with them. If behavior is the same, you make base class and extract duplicated methods there.

So, my general recommendation: dont generalize data members unless you generalize linked behavior

P.S. Tony Hopkinson put it damn straight right

Roman Saveljev
  • 2,544
  • 1
  • 21
  • 20
  • Another form of reuse abuse this sort of thing as far as I'm concerned. Defining the common class / interface will cost you more of every type of resource than three integers, unless you are talking a huge amount of intances, and why would you have that many subcategories of anything? – Tony Hopkinson Aug 09 '12 at 21:58