1

So we have a class that does needs to output the result of an operation. Now this was tightly-coupled to emails, however with dependency injection I thought I could add more persistence options, eg. save to disk.

The problem is that saving to disk requires a path, while 'saving' as an email requires other details (from, to, etc).

Is this something that can be achieved through dependency injection? Or am I doing the whole thing wrong? Check code below and my comments to better understand my problem...

public class OriginalClass
{
    IPersistence _persistence;

    public OriginalClass(IPersistence persistence)
    {
        this._persistence = persistence;
    }

    public void DoSomething()
    {
        // I have all the information needed to send an email / save to disk. But how do I supply it?
        this._persistence.Put("Message to save");
    }
}

public interface IPersistence
{
    bool Put<T>(T data);
}

public class EmailPersistence : IPersistence
{
    public bool Put<T>(T data)
    {
        // How am I going to get the FROM and TO details?
        return EmailManager.Send("FROM", "TO", data.ToString());
    };
}

public class DiskPersistence : IPersistence
{
    public bool Put<T>(T data)
    {
        // How am I going to get the SAVE PATH details?
        // I just used a new initialization. So I'm probably doing this wrong as well...
        new System.IO.StreamWriter("SAVE PATH").Write(data.ToString());

        return true;
    }
}
Zuiq Pazu
  • 285
  • 1
  • 4
  • 12
  • 1
    Whenever you need a runtime paramater to construct an object you need an abstract factory, see http://stackoverflow.com/questions/1943576/is-there-a-pattern-for-initializing-objects-created-via-a-di-container – reggaeguitar Nov 12 '14 at 15:18
  • Why are you using a generic `T` inside your concrete implementation instead of a concrete type? – Yuval Itzchakov Nov 12 '14 at 15:20
  • Do you need that path to be method parameter (I mean change for method calls) or application level only one value? – Sriram Sakthivel Nov 12 '14 at 15:21
  • @YuvalItzchakov - I could have used string, since I am using the ToString() on T, but for now let's assume I had some more intelligent reason for the generic use. – Zuiq Pazu Nov 12 '14 at 15:22
  • 1
    Can you pass the path or the from and to to your IPersistence objects when you construct them? – juharr Nov 12 '14 at 15:23
  • How about using an `IPersistance`, and then implementing it as a concrete type, such as `IPersistance`? – Yuval Itzchakov Nov 12 '14 at 15:23
  • If you can pass the path to IPersistence objects, than make an abstract factory to construct them with a string parameter – reggaeguitar Nov 12 '14 at 15:23
  • @SriramSakthivel - If I understand your question correctly, it could go either way. Either SAVE PATH is something that changes or a constant (that does not require a method parameter), still I would have an issue with the EmailPersistence. – Zuiq Pazu Nov 12 '14 at 15:24
  • @YuvalItzchakov - This is probably on the right track. Can you evolve a bit more on it, or provide an answer with some sample code to better illustrate? – Zuiq Pazu Nov 12 '14 at 15:25
  • I'd take the file path in constructor (if not from method parameter), because for other implementations, it makes no sense to have a path. – Sriram Sakthivel Nov 12 '14 at 15:27
  • Can your `OriginalClass` contain a more specific `IPersistance` such as an EmailPersistance? – Yuval Itzchakov Nov 12 '14 at 15:32
  • @SriramSakthivel - Yes, it would make much more sense to put the SAVE PATH as a constructor / method parameter. But this is something specific to the DiskPersistence, and the Put() specified in the interface (which DiskPersistence implements), I could not request that SAVE PATH parameter. – Zuiq Pazu Nov 12 '14 at 15:32
  • @YuvalItzchakov - As I said, OriginalClass originally just sent the emails. However now that I'm looking into dependency injection, I thought it would make more sense to leave this detail as something that can change. Current use-case is just email, but I would not be surprised if soon saving to disk would be needed. – Zuiq Pazu Nov 12 '14 at 15:34
  • But then you would have to pass different parameters to your interface type. That means you will have to inject a different implementation of `IPersistance` to your `OriginalClass`. – Yuval Itzchakov Nov 12 '14 at 15:35
  • @YuvalItzchakov - I'm new to this whole dependency injection thing. However I've checked about it and it seems to suggest that the dependencies can be changed at runtime - which provides other benefits - such as using TestPersistence for test cases instead of actually saving to disk / emailing, but I wanted to apply it to something real, and I've already stumbled upon this... – Zuiq Pazu Nov 12 '14 at 15:38
  • 1
    Check out [Guide to DI with Unity](http://msdn.microsoft.com/en-us/library/dn223671%28v=pandp.30%29.aspx) - it covers basics with samples, first 4 chapters apply to most containers. – Alexei Levenkov Nov 12 '14 at 15:41

2 Answers2

2

What you need to do is pass 'just enough' contextual information about the message to the persistence class. Passing on email-specific information like from and to however, causes you to leak implementation details of the persistence mechanism into OriginalClass, which is not something you should want. Doing this will cause you to have to change the OriginalClass everytime you add a new IPersistence implementation. This is obviously bad (it breaks both OCP and DIP).

So what exactly to supply is something only you can determine, but it could be something identifier that allows an implementation to retrieve the required information to operate. This could be something like the ID of the Contactperson or organization for who the message is written. This way you only have to pass in the message and this ID and the implementation can use this ID to query the database to get whatever it needs.

However, if these values do not change during the application's runtime, the solution is completely different. In that case you should simply use constructor injection:

public class EmailPersistence : IPersistence {
    private readonly MailAddress from;
    private readonly MailAddress to;
    public EmailPersistence(MailAddress from, MailAddress to) {
        this.from = from;
        this.to = to;
    }

    public bool Put(string data) {
        // How am I going to get the FROM and TO details?
        return EmailManager.Send(this.from, this.to, data.ToString());
    };
}

Since the settings do not change, you can load them from the config file (or from anywhere) during application startup and can simply create a new EmailPersistence using these fixed configuration values.

Steven
  • 166,672
  • 24
  • 332
  • 435
  • Thank you for the detailed response. Unfortunately I cannot use the code sample you provided, since the FROM, TO / SAVE PATH are details that change during the application runtime. I was considering what you mentioned about the ID. But by doing so, would I not be limiting the Persistence classes to being usable only when a contact-person is available. And besides, would I not be breaking the Single-Responsibility Principle if I would set the Persistence classes to retrieve the contact-person by Id and then extract it's details, if all it really needs is FROM, TO / SAVE PATH? – Zuiq Pazu Nov 12 '14 at 16:15
  • @ZuiqPazu: You don't have to break SRP by doing what I suggested, it's just a matter of separating the person-specific stuff from the very generic persistence stuff, so your `OriginalClass` can depend upon an `IContactPersonRelatedPersistar` or something similar. – Steven Nov 12 '14 at 17:52
0

Something like this should work, As now IEmailManager can also go via the DI framework, all you need to do is to bootstrap the EmailManager Construction.

   public class OriginalClass
{
    IPersistence _persistence;

    public OriginalClass(IPersistence persistence)
    {
        this._persistence = persistence;
    }

    public void DoSomething()
    {
        // I have all the information needed to send an email / save to disk. But how do I supply it?
        this._persistence.Put("Message to save");
    }
}

public interface IPersistence
{
    bool Put<T>(T data);
}

public class EmailPersistence : IPersistence
{
    private readonly IEmailManager _manager;

    public EmailPersistence(IEmailManager manager)
    {
        _manager = manager;
    }

    public bool Put<T>(T data)
    {
        // How am I going to get the FROM and TO details?
        return _manager.Send();
    }
}

public class EmailManager : IEmailManager
{
    public string From { get; set; }
    public string To { get; set; }


    public bool Send()
    {
        throw new NotImplementedException();
    }


    public dynamic Data { get; set; }

}

public interface IEmailManager
{
    string From { get; set; }
    string To { get; set; }

    dynamic Data { get; set; }
    bool Send();
}

public class DiskPersistence : IPersistence
{
    public string Path { get; set; }

    public DiskPersistence(string path)
    {
        Path = path;
    }

    public bool Put<T>(T data)
    {
        // How am I going to get the SAVE PATH details?
        // I just used a new initialization. So I'm probably doing this wrong as well...
        new System.IO.StreamWriter(Path).Write(data.ToString());

        return true;
    }
}
Preet Singh
  • 1,791
  • 13
  • 16
  • DiskPersistence can be adjusted as you suggested. However the EmailPersistence has a different TO (and possibly FROM), and I don't quite like having to change the IEmailManager property all the time. Any other suggestions? – Zuiq Pazu Nov 13 '14 at 08:20
  • Sorry Buddy, didnt quite get what you asked in the above comment – Preet Singh Nov 14 '14 at 10:16