2

Is it ok to have subclass which does not declare its own variables (instance variables) and inherits from super class or base class? It has methods and there are some common methods which it overides but do not have variables of its own.

Is it ok to have subclasses which defines behavior only or I am missing something? I want to divide in subclasses to separate logic of my classes. I have Medication class and I want to subclass to DailyMedication , WeeklyMedication but I have all my instance variable in Medication as I do not have any specific for these classes. I want to make subclasses to divide logic as I have too much if conditions in my code.

MedicationEntity
name
dose
weekdays    //in case of daily and monthly this is null 
monthdays   //in case of weekly and daily this is null
frequencyType  //Daily or weekly or monthly
etc

EDIT: I want to make subclasses because these all classes have diffrent logic.I have enum of frequency(for Daily,Weekly etc) in my Medication class and this makes worse as i have to put all the checks in my code for these frequency type as if frequency is that than do that and if daily than do that as now i want to remove these switch conditions.

1)Also please suggest any good idea as weekdays is null if frequency is daily or monthly.But i need to send null to server.So where i can put weekdays if i subclass as it is not null only in case of weekly frequency but as i have to send null to server in request for all other frequencies.Should i put weekdays in subclass if yes than how i send null to server if frequency is daily as DailyMedication have no information about weekdays.

3
  • 3
    This question combined with weekdays, monthdays and frequency has a code smell to me, which makes me suspect the root cause of the question is the underlying data representation could be improved. If you had a class and a couple of sub classes for frequency would the problem go away?
    – mattnz
    Jul 14, 2014 at 21:16
  • @mattnz you are right i am having switch case code smell.Now i want to refactor it.can you please suggest something if i subclass the Medication for point 1.Thanks for all your help
    – codester
    Jul 14, 2014 at 21:25
  • Why not let the frequency be freeform? Some medications require by-weekly administration. Others are every 4 days. How do you account for those? Jul 15, 2014 at 1:45

4 Answers 4

1

Having three separate classes might make sense if you know you're going to add some special members to some of these subclasses and you just haven't gotten around to it yet (but you're going to really soon, right?).

Normally, I'd agree with the senior developer: This seems like an unecessary complication. Could you create an enumeration called Frequency and use that to determine which type of Medication you're dealing with, something like this pseudo-code:

Enum MedicationFrequency{MONTHLY, DAILY, WEEKLY, HOURLY}

class Medication
{
    private MedicationFrequency frequency;
    ...
}

I suppose if you really want to keep the different implementations completely separate, you could try to inject specific algorithms into the Medication instances. Maybe something like this:

Interface MedicationProcess
{
   public void doSomething();
   public String getWhen();
   public MedicationFrequency getFreq();
}

class WeeklyMedProcess implements MedicationProcess
{
   String weekdays;
   public MedicationFrequency getFreqy() { return MedicationFrequency.WEEKLY;}
   public String getWhen() { return weekdays; }
   //...
}

class DailyMedProcess implements MedicationProcess
{
   public MedicationFrequency getFreqy() { return MedicationFrequency.DAILY;}
   //...
}

class MonthlyMedProcessimplements MedicationProcess
{
   String monthdays;
   public MedicationFrequency getFreqy() { return MedicationFrequency.MONTHLY;}
   public String getWhen() { return monthdays; }
   //...
}

class Medication
{
    private MedicationProcess someProcess;
    private String updateColumnName;

    public void setUpdateColumnName(String s)
    {
        this.updateColumnName= s;
    }

    public void setProcess(MedicationProcess p)
    {
        this.someProcess = p;
    }

    public doMedicationThing()
    {
        this.someProcess.doSomething();
    }
}

Doing this in Java, You'd have a Spring configuration that might look like this:

<bean id="dailyMedProc" class="DailyMedProcess/>
<bean id="weeklyMedProc" class="WeeklyMedProcess/>

<bean id="medication1" class="Medication">
    <property name="process" ref="dailyMedProc/>
    <property name="updateColumnName" value="weekdays"/>
</bean>

<bean id="medication" class="Medication">
    <property name="process" ref="weeklyMedProc/>
    <property name="updateColumnName" value="monthdays"/>
</bean>

To answer your quesion about weekdays and monthdays, there are many ways to do this and it's not very clear from your question just what weekdays and monthdays are. Since you show them to be single variables, I'll assume they are just comma-separate strings.

It sounds like you also have columns in some table named weekdays and monthdays which need to be set to null if there is no value. Since you already know the frequency of the Medication (via its someProcess.getFreq), you know which field to set to null. The implementors of MedicationProcess can be required to implement getWhen, which will return the comma-separated list of days/month-days. With these two pieces of information, you can easily update the correct columns.

Having Medication have knowledge over what to do when specific MedicationFrequencies are encountered does break the design pattern a little bit. It's not great, but it's a simple solution.

If you don't like that, you could include the column name that should be updated for a specific instance of Medication. Only slightly better - I don't like mixing that sort of low-level data access into such classes, but I haven't seen the rest of your design, so maybe it's OK here...

Another option would be to have the MedicationProcess do its own database insert/update. This is a little trickier and I can't really give any concrete examples of how to do this without knowing the details about your schema, though I think this would be the best solution, if you can implement it.

There are other ways to implement the strategy pattern, depending on the tools and frameworks available to you.

Also, see: https://stackoverflow.com/questions/17721623/advantages-of-using-strategy-pattern-in-php The question is PHP-specific, but the answer is not.

12
  • Thanks @FrustatedWithFormsDesign for your reply.But i have done exactly that having enum MedicationFrequency for type.Now my problem is that i am having too much if conditions floating around my project for these frequencies so i want to seprate the logic for these subtypes.Do you have any better solution and can you please explain why you agree with my senior developer as i do not understand its concept.Thanks for your help.
    – codester
    Jul 14, 2014 at 20:44
  • @codester: see my update. Jul 14, 2014 at 20:51
  • @ FrustratedWithFormsDesigner thanks for all your help, can you please explain little bit why strategy pattern has benifit over subclassing and can you please suggest something for weekdays null value i have edited my post.Thanks FrustratedWithFormsDesigner for all you help
    – codester
    Jul 14, 2014 at 21:07
  • @codester: The Strategy Pattern only has one Medication class in the system. Different frequency-dependent implementations depend on the classes which implement MedicationProcess, but you only have to deal with different process types when you create an instance of Medication - everywhere else, your code only needs to be aware of Medication - no subtypes. The senior dev you spoke to is correct - unncessary classes will eventually add unnecessary complexity. It also makes it easier to add new frequencies in the future: the only old code that will may to change is the config/setup. Jul 14, 2014 at 21:22
  • @codester: As far as weekdays and monthdays go, it might be possible to move these to WeeklyMedProcess and MonthlyMedProcess. That would probably be ideal but I don't know how easy that would be without seeing all of your code. Here's the thing: does the Medication class really need to be aware of such things, or could it simply query its internal MedicationProcess for such implementation-specific details? I can't answer such things without seeing much of your code, but think about that question... ;) Jul 14, 2014 at 21:25
1

I can see this both ways:

  • make the sub classes: If the sub classes have truly different logic, then by all means that logic should be separated even if there are no instance variables. Think of the classic OOP "hello world" - a drawing program with Shape, and different subclasses for Square and Triangle. CalculateArea() would be different, but they'd both work on height and width.
  • keep them together: In your example, I might have one class Medication with a property that represents the idea of an Interval or Duration. In an ideal world you would have just one set of logic that would operate on an "Interval" without caring very much if it was every 1 hour, every 24, or every 168.

Without more information my suspicion is that your senior dev is correct for your particular situation. (That's how senior devs get to be senior devs - by being right more often than they're wrong).

I think you're right to want to remove all that conditional code. The question is whether it's better to hide the conditions in subclasses or to re-think the process so the interval becomes just a simple parameter.

Look at this another way: what would you have to do if your boss asked you to support a new time period (Daily, Weekly, Monthly, Every17Hours, Every3.5Days, etc.)? Can you code your methods so instead of looking at enums you can just work off the interval?

It may be necessary to talk to the senior dev & see what he has in mind. It's possible his concern is based on an insufficient grasp of the issues.

3
  • No these all classes frequencies have diffrent logic.
    – codester
    Jul 14, 2014 at 20:50
  • thanks for all your help.I have edited my post can you please suggest anything for null values(weekdaysetc).can i do anything about them or i should keep in Base class.No we cannot handle with interval.I think about subclassing
    – codester
    Jul 14, 2014 at 21:04
  • can you please suggest something for null values of weekdays and how can i send it to server when frequency is other than weekly.Please suggest something for point 1 if i subclass the frequencies.
    – codester
    Jul 14, 2014 at 21:23
0

Sometimes it makes sense to have a class hierarchy in which derived classes differ only in implementation methods from base classes. In particular, if you have a hierarchy of objects that have the same inputs, the same outputs, and interact in identical ways with the rest of the system, but nevertheless have substantially different implementation logic, you can consider using a method-only hierarchy.

For instance, consider CAD software which allows users to add "measurement" objects to its database. Each measurement entity holds a persistent reference to one or more geometric entities, and outputs a real number (the measurement) in a specified unit system. Now, there might be many different measurement algorithms -- arc length, max/min radius of curvature or extrema along a vector of selected curves; surface area or perimeter of selected faces; and so on. Each release of the CAD system is likely to add new measurement types. But they should all behave identically to the rest of the system. E.g. if geometric entities are copied and pasted, their measurements should be copied alongside with references updated. Or if a geometric entity is highlighted, perhaps its measurements should be queried from the database and shown in a window. As long as there is substantial identical code dealing with the generic aspects of measurement entities, a method-only class hierarchy (probably mirroring an interface hierarchy) is a sensible implementation.

In your case, however, you are creating a top-level class which is the union of other classes with substantially different inputs or outputs, like so:

public abstract class Top
{
    public abstract double? Sub1Properties { get; }

    public abstract double? Sub2Properties { get; }
}

public class Sub1 : Top 
{
    double data = 24.0 // or whatever.
    public override double? Sub1Properties
    {
        get
        {
            return data;
        }
    }
    public override double? Sub2Properties
    {
        get
        {
            return null;
        }
    }
}

This, I think is unfortunate. Consumers of your classes will likely write code like so:

        if (top.Sub1Properties != null)
        {
            // DO something
        }
        else if (top.Sub2Properties != null)
        {
            // Do something
        }

Now imagine that, tomorrow, you need to add a third subclass:

public abstract class Top
{
    public abstract double? Sub1Properties { get; set; }

    public abstract double? Sub2Properties { get; set; }

    public abstract double? Sub3Properties { get; set; }
}

All those if statements will need to be updated. This is old-time procedural logic, and is IMHO an anti-pattern. In your case, if you need to add a new dosing regimen (bi-monthly) in a later software release all those if statements would need updating.

In your case, I might suggest dividing up "Medication" and "Dosing Schedule" into separate objects/tables, with each medication having a link to its appropriate schedule. Then adding a new regimen would require only a new entry in the table. I might also represent frequencies in a generic way to the outside world, for instance by using a TimeSpan. If there is substantial logic specific to each regimen, it could be represented as a hierarchy of classes, as long as there is a generic representation to the outside world for all the data therein.

2
  • I can not change data model of server as it is on server.So i always need to send null values for weekdays for other frequency.But i need to manage this here.That is real problem i need to check null.Thats why i ask question.Can you suggest any other way? I cannot make instance by timespan lots of going in the things with medication object
    – codester
    Jul 15, 2014 at 4:51
  • OK, now I see your difficulty -- you can't actually change the underlying data model at all. You can only change how it is presented.
    – dbc
    Jul 15, 2014 at 6:25
0

Take 2.

I understand that you have the following constraints:

  1. You can't change the underlying data model on the server at all. The server-side representation of "MedicationEntity" has hardcoded fields corresponding a small number of possible sets of dosing schemes, and you need to be able to translate from & to this model.

  2. The various dosage schemes can't be generalized as a set of standard time intervals. E.g. "M, Th, S" might be a valid weekly dosage schedule. This lack of perfect uniformity kills the idea of representing all the frequencies as a simple TimeSpan.

In that case, I would suggest that your "MedicationEntity" class should completely hide the server-side dosage frequency fields. It must needs remember them internally, since that is what is stored on the server, but it should never present them as public properties. Instead it should have some property "DosageFrequency" which returns an instance of a class implementing an interface "IDosageFrequency" (here I am using c# because I am most familiar with it):

public interface IMedicationEntity
{
    string Name { get; }

    double Dose { get; }

    string Units { get; } // 

    IDosageFrequency DosageFrequency { get; set; }
}

The "IDosageFrequency" interface could encapsulate all the business logic for each dosage frequency and present them in a neutral manner, e.g.:

public interface IDosageFrequency
{
    string Name { get; }

    string Description { get; } // Localized for presentation in the user interface.

    string FormattedPromptString(IMedicationEntity medication); // E.g.: string.Format("{0} {1} of {2} three times a week.", med.Dose, med.Units, med.Name)

    DateTime NextDoseTime(DateTime now);

    TimeSpan DoseTimeRequiredAccuracy { get; }

    IEnumerable<DateTime> DoseTimesBetween(DateTime start, DateTime end);

    // etc
}

You could then implement this interface with different classes corresponding to each basic dosage scheme. You could also use the abstract factory pattern to maintain libraries of standard DosageFrequency objects to present in the user interface, to allow end users to select a new dosage scheme for some pre-existing or new medication.

This would leave you with only two nasty switch statements, both inside your "MedicationEntity" class:

  1. Constructing the appropriate IDosageFrequency from the stored fields from the server, AND

  2. Selecting the appropriate server fields when a new IDosageFrequency object is set.

By hiding the sever-level hardcoded dosage schemes, this pattern (is there a name for it?) could keep the switch statements from growing out of control while also avoiding the need to create different medication class types for different dosage schemes.

Update: incidentally, I just noticed a potential pitfall with your scheme of creating a class hierarchy for the "MedicationEntity: objects. If you need to implement the ability to change the dosage scheme for a pre-existing MedicationEntity, you will need to immediately locally replace the existing MedicationEntity with an instance of a different subclass, e.g. DailyMedicationEntity -> WeeklyMedicationEntity; if you didn't you would be unable to locally work with the new settings. But the server side representation of the MedicationEntity would not need to be deleted and recreated, which means you temporarily could have obsolete, replaced MedicationEntity instances floating around in your client referring to still-extant database entities. This, I suspect, might Cause Problems.

1
  • It seems like you have a great answer.
    – InformedA
    Jul 15, 2014 at 7:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.