Constructor injection with two different interfaces (Single Responsibility and Interface Segregation)

Question

I am learning SOLID Principles. I am working now with Dependency Injection and Interface Segregation Principles. I've already got the basics of this two but when I combined it, I got confused. Here's my implementation..

class Person
{
    public Person(string name, int age)
    {
        Name = name;
        Age = age;
    }

    public string Name { get; set; }
    public int Age { get; set; }
}

class DisplayPerson
{
    private readonly IWeapon iwep;
    private readonly Person pers;
    public DisplayPerson(IWeapon iwep, Person perss)
    {
        this.iwep = iwep;
        this.pers = perss;
    }

    public void DisplayAll()
    {
        Console.WriteLine("Name: " + pers.Name + "\nAge: " + pers.Age + "\n" + "Weapon: "
            + iwep.weaponName() + "\nDamage: " + iwep.damage().ToString());
    }
}

I created another class for displaying information as "S" rule in SOLID says that a class shouldn't do which it is not supposed to do. And I think displaying that information is not its task. (Please correct me if I'm wrong)

class Sword : IWeapon
{
    private string weaponNames;
    private int damages;
    public Sword(string weaponName, int damage)
    {
        this.weaponNames = weaponName;
        this.damages = damage;
    }

    public string weaponName(){ return this.weaponNames; }

    public int damage(){ return this.damages; }
}

public interface IWeapon
{
    string weaponName();
    int damage();
}

With this interface IWeapon, I can now able to add many weapons that have weaponName and damage. But if I want to add another weapon with added functions, we have to follow the ISP principle, right? So I created this interface and class below.

class Gun : IWeaponV1
{
    private string weaponNames;
    private int damages;
    private int range;

    public Gun(string weaponName, int damage, int range)
    {
        this.weaponNames = weaponName;
        this.damages = damage;
        this.range = range;
    }

    public string weaponName() { return this.weaponNames; }
    public int damage(){ return this.damages; }
    public int ranges() { return this.range; }
}

public interface IWeaponv1 : IWeapon
{
    int range();
}

And I implemented it like this..

    static void Main(string[] args)
    {
        DisplayPerson disp = new DisplayPerson(new Sword("Samurai Sword", 100), new Person("Jheremy", 19));
        disp.DisplayAll();
        Console.ReadKey();
    }

My problem is how do I inject IWeaponv1 to DisplayPerson class above? Is it possible or my understanding in SOLID principles is just wrong. Please correct me if you see that I've done something wrong or bad practice :)

Answer 1

The implementation that you present in your question looks good from a theoretical point of view; however, it does have some design issues in the real world.

1. While the Single Responsibility Principle does promote more cohesive classes, it should not be extended to a level where every functionality is moved to a new class. Case in point is the DisplayPerson class which is not required at all (or should be implemented differently which I will discuss shortly). Classes represent real world objects. In the real world, if you ask a person their name, they would tell you their name rather than redirect you to another person who tells you their name. A Person object is therefore allowed to have a method for printing it's own properties and this does not defy the Single Responsibility Principle in any way. Same goes for the weapons as well.

2. Consider that you want to print the range of a weapon if the weapon supports this functionality/behavior. As it stands, the DisplayPerson has-a IWeapon reference but IWeapon does not have ranges as part of it's contract. How then do you print the range of a weapon? (range will not be accessible through iwep reference in the DisplayAll method). You will have to use a typeOfcheck and then downcast IWeapon to IWeaponV1 so that you can call the ranges method. The problem with this approach is that DisplayPerson will now have to work with two different interfaces rather than working with a single interface. (Not to mention the conditional checks that defy the purpose of using a common interface)

Keeping the above points in mind, you can make the following code changes :

1. Add a display method to IWeapon interface and Person class.
2. Print the properties of the person in the display method of Person class. Similarly, print the properites of a weapon in the display method of the individual weapon implementation

With the above changes, you can call display directly on your Person or IWeapon objects in the DisplayPerson class by relying on a single interface ( without violating the Single Responsibility Principle) :

public void DisplayAll() {
        Console.WriteLine("Person " + pers.display() + "\n has weapon: " + iwep.display());
}

Answer 2

It's sometimes confusing with far you can follow Single responsibility principle, so its alright.

There are a couple of issues on the code I would like to point out before answering your questions.

1. WeaponName, Damage and Range are data, they are not functionality. They should be declared as property and not as method.
2. Gun should implement IWeaponv1 . Which I think you missed it by mistake.
3. All the properties are set in constructor,i.e. constructor injection. You dont need a private set in that case for Person class, or any other classes

Finally to your actual question

DisplayPerson disp = new DisplayPerson(new Sword("Samurai Sword", 100), new Person("Jheremy", 19));

DisplayPerson disp1 = new DisplayPerson(new Gun("Shotgun Axe", 100,100), new Person("Matt", 22);
//Assuming you have implemented Gun:IWeaponv1 

This looks fine, it doesn't violate single responsibility principle. You are overthinking it in this simple example. In the above example, only one class(Display person) with functionality exists and rest of them are just holding data. That's a good thing when you separate data from functionality. A simple way to check if the Single responsibility is broken is by checking the method name against the name of the class. If it's out of place then you are violating SRP.