How can i avoid breaking LSP in this example? C#

Question

I have a base class called Message like this:

public abstract class Message { protected int m_id; protected bool m_localized; protected string m_metaData; public int GetID() { return m_id; } public bool GetLocalized() { return m_localized; } public string GetMetadata() { return m_metaData; } }

Then, i have two more classes that inherit from Message for example:

public class ClassicMessage : Message { private string m_title; private string m_content; public void SetTitle(string title) { m_title = title; } public void SetContent(string content) { m_content = content; } public string GetTitle() { return m_title; } public string GetContent() { return m_content; } } public class MessageWithCustomContent : Message { private List<CustomContent> m_content; public MessageWithCustomContent() { m_content = new List<CustomContent>(); } public List<CustomContent> GetContent() { return m_content; } public CustomContent GetContentEntry(int id) { return m_content.find(x => x.ID.Equals(id)); } } public class CustomContent { private int m_id; public int ID { get; set { m_id = value; } } private string m_body; public string Body { get { return m_body; } set { m_body = value; } private Image m_image; public Image Image { get { return m_image; } set { m_image = value; } } }

In a case like this, how can i unify the app interface if the derived classes has similar methods but these methods have different return types? (even when the methods try to do the same)

I know that with the example i'm breaking the Liskov Substitution Principle and the Open/Closed principle, what's the best approach to get around with that?

Thanks for your help!

Edit:

For more clarity, what i'm trying to achieve is to create a common interface to manage all the possible messages as the base "Message", because i want to avoid using typeof in the consumer class.

for example:

if(message is MessageWithCustomContent) { // do something with the contents. } else if(message is MessageWithCustomContent) { // do another thing with the contents. } etc...

Coolio · Accepted Answer · 2017-02-06 01:26:31Z

~~You could change Message to be generic, and the T would specify the Content return type. See example below.~~

Edit You could use a "IMessage" and a "Message: IMessage" as base. You would then be able to create a IMessage list like so

var messages = new List<IMessage> { new ClassicMessage(), new MessageWithCustomContent() }; foreach (var message in messages) { message.GetContent(); }

Below is how the implementation of IMessagecould be done.

public interface IMessage { int GetID(); bool GetLocalized(); string GetMetadata(); object GetContent(); } public abstract class Message<T> : IMessage { protected int m_id; protected bool m_localized; protected string m_metaData; public int GetID() { return m_id; } public bool GetLocalized() { return m_localized; } public string GetMetadata() { return m_metaData; } object IMessage.GetContent() { return GetContent(); } public abstract T GetContent(); } public class ClassicMessage : Message<string> { private string m_title; private string m_content; public void SetTitle(string title) { m_title = title; } public void SetContent(string content) { m_content = content; } public string GetTitle() { return m_title; } public override string GetContent() { return m_content; } } public class MessageWithCustomContent : Message<List<CustomContent>> { private List<CustomContent> m_content; public MessageWithCustomContent() { m_content = new List<CustomContent>(); } public CustomContent GetCustomContent(int id) { return null; } public override List<CustomContent> GetContent() { return m_content; } } public class CustomContent { private int m_id; public int ID { get; set; } private string m_body; public string Body { get { return m_body; } set { m_body = value; } } }

It's a good idea but then i cannot create a single list of messages to manage them because i have to specify the generic type in the declaration
Nice! i came to a very similar implementation by myself. But it seem like the approach that i'm using for this problem is wrongly designed, because then there's no way to handle the content without checking if it's a list or a string. I'll mark your answer as correct anyways. Thank you very much for your help!

CodingYoshi · Accepted Answer · 2017-02-06 01:50:26Z

I will explain how you break LSP below but before I do that, you are not really doing any inheriting. Yes you are declaring your classes to be inheriting but you are not really inheriting anything. So before learning LSP, perhaps you need to get a grip on inheritance firstly.

How do I know if I am breaking LSP?

Lest say your Message class was like this, notice the virtual and abstract methods:

public abstract class Message { protected int m_id; protected bool m_localized; protected string m_metaData; public virtual int GetID() { return m_id; } public virtual bool GetLocalized() { return m_localized; } public abstract string GetMetadata(); }

Create a list like this:

var messages = new List<Message>();

Then add concrete types to that list of all the inheriting types. Then do this:

foreach(var thisMessage in messages) { var id = thisMessage.GetID(); var loc = GetLocalized(); var meta = GetMetadata(); }

If you get no exception thrown because one of the inheriting classes decided it does not need one of those methods, then you have not broken LSP. The idea is that if something is inheriting Message, then it should inherit everything. Otherwise, we cannot safely and with confidence substitute the inherited one for the parent one.

The reason this principle is important is because there may be existing code which is using Message, as shown in the foreach above, where it is treating all the types polymorphically and a developer decides to inherit it like this:

public abstract class BadMessage { public override int GetID() { throw new InvalidOperationException ("This method is not needed for BadMessage and should not be called"); } public override bool GetLocalized() { ... } public override string GetMetadata() { ... } }

You see this will break existing code. And the worst part is, the compiler will not even be able to catch it, until it surfaces like an ugly bug in production.

JHBonarius · Accepted Answer · 2017-02-05 18:48:11Z

Well, you're missing the interface methods in de base class. Abstract functions, that get implemented in the derivative classes. If you get a Message, not knowing what kind it is, how would you request its contents? You could add derivative-specific methods to your base, but you'd have to implement an not_implemented exception in a virtual implementation in the base class to compensate for all derivatives not implementing it, and add exception handling. But then you should ask yourself: " is this class really a derivative? What do I want to achieve."

Collectives™ on Stack Overflow

How can i avoid breaking LSP in this example? C#

3 Answers 3

2 Comments

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

Comments

Related