The issue of designing polymorphism

Question

First of all, sorry for the long question, but I could not write it shorter :)

Real world example: we have a large roll of paper on which small “stickers” are printed. Each sticker has a code. The first two letters of the code tell us which sticker it is (a sticker representing the new roll, a sticker representing the end of the current roll, a sticker that should go to quality control ... but most of them are ordinary enumerated stickers).

For example, a sticker with the code XX0001 means that after it there should be only normal enumerable codes (for example, from NN0001 to NN9999), always the same number. Code QC0001 tells us that the following 10 codes (from QC0001 to QC0010) should go for quality control.

I developed the application so that each type of sticker was its own class - NormalSticker , BadSticker , ControllSticker , QualitySticker , ... They all inherit from the SticerBase class, which contains some common data for all of them (scan quality, date and time of scanning, code content). Instances of these classes are created in the static Parser class, which checks the code and returns the corresponding object to us.

Everything is working fine, but now I have stopped. I also have a Roll class that has a set of stickers implemented as a List<StickerBase>. This class has the public AddSticker(StickerBase) method, with which we add stickers to the roll. But this method should contain some logic, for example, if we get the code XX001, then the next 9999 stickers should be from NN0001 to NN9999. The only option I see here is to make desicions based on the type of sticker, for example:

 public void AddSticker(StickerBase sticker) { if (sticker.GetType().Equals(typeof(StickerNewRoll))) { // Next 9999 sticker should be in the form of NN0001 to NN9999 } if (sticker.GetType().Equals(typeof(EnumeratedSticker))) { // Add 9999 stickers to the list, other business logic... } if (sticker.GetType().Equals(typeof(QualitySticker))) { // Stop the machine and notify the worker } } 

I would be very surprised if this is the right approach. Any ideas?

Change is a possible solution: because for each sticker I know what the next should look like, I can add a new public Sticker NextStickerShouldLookLike() method public Sticker NextStickerShouldLookLike() for each Sticker class. In the validation logic (similar to the Péter Török answer ) I can simply check if the current sticker matches previousSticker.NextStickerShouldLookLike() . The Validate method would have two input parameters - the current and the previous sticker.

Answer 1

I can see how rage arises in other answers, but I'm not sure that you can come to a good solution without studying your example in more detail.

Firstly, you have three classes: Roll, Parser, creating stickers and stickers themselves (divided into derived classes). Perhaps there is another class that implements some business logic that you have not mentioned? It may be worth describing ...

First question: Given that some class is responsible for attaching the stickers to the roll (Parser class?), Can you leave the roll completely amivalent, on which stickers does it receive, and put your logic in another place? Is the logic of what to do when a sticker is type X something for a roll to be aware of, given that this is not a roll that goes out and receives the stickers?

Second: How polymorphic are your stickers? Do they have different methods? Do they have different properties? Or are they similar enough that you can just put the shortcut in the StickerBase class?

Third: is the sticker worth telling what to do? I.E. if the roll should call the sticker.TellMeWhatToDoNextPlease() method (implemented as a virtual method and excluded from the derived sticker classes), moreover, the roll is not responsible for what people try to stick to it. You can ask the same question about the class that is responsible for applying the stickers on the roll. You could train the monkey to do this, and let the roulette understand it, or you can put your logic there (if the labels are made, she should already know what she is doing) and let the roll accept what it’s stuck from.

Basically - what controls your process? A roll of paper sticking stickers on it, stickers themselves, or something else doing and sticking stickers?

Answer 2

Do you want to add a set of stickers associated with a specific sticker in one go, or do you want to verify that the added stickers comply with the restrictions set by the last special sticker?

In the first case, you can add the polymorphic GetAssociatedStickers() method to your sticker classes, which returns a set of stickers from NN0001 to NN9999 to a sticker with code XX001, etc. You can then add this set of stickers immediately after the control sticker.

Update

For verification, you can have a new StickerValidator interface and GetValidator method in sticker classes. Special stickers will return the correct validator object (which can be implemented as an anonymous class or inner class), while regular stickers will return null . Then AddSticker can be changed to look like

 public void AddSticker(StickerBase sticker) { if (sticker.GetValidator() != null) { this.validator = sticker.GetValidator(); // add the sticker to the list } else { if (this.validator == null || this.validator.validate(sticker)) { // add the sticker to the list } else { // set error state } } } 

Answer 3

A conditional expression based on a type is antipattern, and you are correct that you should try to avoid it. One problem, for example, is that you will have to update this method when creating a new subclass of StickerBase . Another problem is that the method signature signals that the caller can pass any StickerBase implementation, but in fact only a few are supported.

If possible, put your logic in the Sticker implementation. Have an abstract method in the StickerBase class and override in subclasses. Thus, you only need to call the method call in the AddSticker method and you will not need to know what type of stickers are added.

If this is not possible, and you really need your code to deal with the different types of labels in your Roll class, you might consider looking for a Visitor . Try to avoid this and, if possible, take the first approach.

Answer 4

Create a virtual function GetNextStickerLabel (); and implement it differently in any type of type. Generally, if you need to ask what type is an object and make a decision (if) on it, something is wrong with the design.

Answer 5

It seems that the Visitor sample (possibly with an Iterator drawing) is the ideal candidate here.

I think that accepting the sticker as a first-class citizen and creating your own class is already a good start for an elegant solution in your case.

First you must create a visitor interface (or base class) for your sticker hierarchy.

 public interface IStickerVisitor { void Visit(NewRollSticker sticker); void Visit(EnumeratedSticker sticker); void Visit(QualitySticker sticker); //need a method for every kind of sticker here } 

Then you need to add the abstract Accept method to your sticker class that accepts the Visitor parameter, as shown below;

 public abstract void Accept(IStickerVisitor visitor); 

the contents of this method in specific classes should simply look below:

 public abstract void Accept(IStickerVisitor visitor) { visitor.Visit(this); } 

At this point, you can create a specific visitor, say StickerRollerVisitor , which contains the necessary logic to add stickers to the list you need.

 public class StickerRollerVisitor : IStickerVisitor { private RollList rollList; public StickerRollerVisitor(RollList list) { this.rollList = list; } public void Visit(NewRollSticker sticker) { // Next 9999 sticker should be in the form of NN0001 to NN9999 } public void Visit(EnumeratedSticker sticker) { // Add 9999 stickers to the list, other business logic... } public void Visit(QualitySticker sticker) { // Stop the machine and notify the worker } } 

In this visitor implementation, you can accept an Iterator rather than the original entry list (forward, backward, skip, etc.). ) list with user strategy.

After creating a default StickerRollerVisitor in a RollList or accepting a visitor as a constructor parameter, your code may look like this:

 private StickerRollerVisitor rollListStickerVisitor; public void AddSticker(StickerBase sticker) { sticker.Accept(rollListStickerVisitor) } 

After including this template in your design, you can use it in different ways, creating new visitors, so this will be a great improvement / addition to your design.

Answer 6

I would still rewrite it like this:

 if (sticker is StickerNewRoll) { // Next 9999 sticker should be in the form of NN0001 to NN9999 } else if (sticker is EnumeratedSticker) { // Add 9999 stickers to the list, other business logic... } else if (sticker is QualitySticker) { // Stop the machine and notify the worker } 

...

Answer 7

You have a good option to solve this problem.

You can implement the enumeration of label types and attribute.

 public enum StickerKinds { NewRoll, Enumerated, Quality } public class StickedAttribute : Attribute { public StickedAttribute(StickerKinds kind) { _kind = kind; } private readonly StickerKinds _kind; public StickerKinds Kind { get { return _kind; } } } 

Now you can have a class called "StickerLabeler" that implements the "NextLabel" method, which takes an input parameter of type StickerKinds, and returns a string representing the entire label as text.

And finally, an extension method for easily retrieving attributes:

 public static class ObjectExtensions { public static TAttribute GetAttribute<TAttribute>(this object source) where TAttribute : Attribute { if (source != null) { object[] attributeSearchResult = source.GetType().GetCustomAttributes(typeof(TAttribute), true); if (attributeSearchResult.Length > 0) { return (TAttribute)attributeSearchResult.Single(); } else { return default(TAttribute); } } else { return default(TAttribute); } } } 

Your code will look like this:

 public void AddSticker(StickerBase sticker) { sticker.Label = StickerLabeler.NextLabel(sticker.GetAttribute<StickedAttribute>().Kind); // TODO: Implement code to add the sticker to the store. } 

EDIT: I forgot to mention that you will use StickedAttribute in your StickerBase subclasses:

 [Sticked(StickerKinds.NewRoll)] public class StickerNewRoll { ... } 

Strike>

UPDATE: In fact, there is no reason for a subclass. You may have a sticker class, and this attribute will determine the type of sticker.

UPDATE 2: you can make another improvement. Add the read-only property to the “View” sticker class, which the attribute can read, and it will return the value of the StickerKinds enumeration of the so-called attribute, so now your code can be even cleaner:

 public void AddSticker(StickerBase sticker) { sticker.Label = StickerLabeler.NextLabel(sticker.Kind); // TODO: Implement code to add the sticker to the store. } 

UPDATE 3: Andreas, the commentator of my answer, made me think that you might need a subclassification, because each type of sticker will have its own properties, and the so-called attribute will be applied to these derived classes.