2

I created an ILogger interface and a ListViewLogger class that implements ILogger, now ILogger has a method (SetListViewReference) that shouldn't be there since not all Loggers will log to a ListView, and i don't know where to place it, because i'm doing dependency injection i need that method though, so what should i do ? thanks!

using System;
using System.Windows.Forms;

namespace AutoTweet
{
    public interface ILogger
    {
        void Log(string text);

        void SetListViewReference(ListView listview);
    }

    public class ListViewLogger : ILogger
    {
        private ListView _lvLog;

        public void Log(string text)
        {
            _lvLog?.Items.Add(makeLvi(text));
        }

        public void SetListViewReference(ListView listview)
        {
            _lvLog = listview;
        }

        private ListViewItem makeLvi(string text)
        {
            ListViewItem ret = new ListViewItem { Text = DateTime.Now.ToString() };
            ret.SubItems.Add(text);
            return ret;
        }
    }
}
  • 4
    That method looks like it is the constructor for `listviewlogger` – Maria Ines Parnisari Jan 02 '16 at 16:41
  • @miparnisari I thought about accepting the listview reference from the constructor, but i can't though, because of DI, i don't have the scope of the listview. –  Jan 02 '16 at 16:43
  • 1
    Interfaces ought to exist in order to support client requirements. When would *a client* ever need to invoke `SetListViewReference`? – Mark Seemann Jan 02 '16 at 17:33
  • @MarkSeemann How would my ListViewLogger class log events that are happening in my application without having a reference to the ListView ? –  Jan 02 '16 at 17:35
  • ... let it create it itself, or inject it via its constructor... On a parallel note, please consider this: http://stackoverflow.com/a/7906547/126014 – Mark Seemann Jan 02 '16 at 17:37
  • @MarkSeemann What do you mean by "let it create itself" ? and about injecting it via ctor, as i said in my answer to miparnisari, i can't because the listview is not in scope. –  Jan 02 '16 at 17:50
  • Oh, see this, then: http://stackoverflow.com/a/1945023/126014 – Mark Seemann Jan 02 '16 at 18:20

2 Answers2

1

Have a new IListViewLogger interface which will inherit from ILogger and have that additional method:

public interface ILogger
{
    void Log(string text);
}

public interface IListViewLogger : ILogger
{
    void SetListViewReference(ListView listview);
}

public class ListViewLogger : IListViewLogger
{
    //...
}

The design really depends on the scope of the ListView though.

If there's only one ListView in the application, then it should be set somewhere in the composition root, where you will be able to set it because you know the logger's concrete type. If the scope is per-view, then it's perfectly fine to have this extended interface, since you need to somehow tell the logger where it should log to.

Adi Lester
  • 24,731
  • 12
  • 95
  • 110
  • Haha!, that's exactly what i thought about before asking here this question, but that would defeat the whole purpose of DI, since not all Loggers will implement IListViewLogger. my Form1 ctor needs to accept all type of loggers that implement ILogger, so i cannot make my Form1 ctor accept IListViewLogger. –  Jan 02 '16 at 17:02
1

If you would like to remove dependency on UI control from ILogger interface you can apply dependency inversion principle. One way is to add an event to the interface. For simplicity sake I'll not introduce another interface i.e.:

interface ILogger
{
    void Log(string msg);
    event Action<string> LogMessages;
}

class PublisherLogger : ILogger
{
    public event Action<string> LogMessages = (msg) => { };

    public void Log(string msg)
    {
        Console.WriteLine(msg);
        LogMessages(msg);
    }
}

And then use it inside your Form control:

public Form1()         
{        
    InitializeComponent(); 
    var publisherLogger = new PublisherLogger(); //or inject it
    publisherLogger.LogMessages += msg =>
    {
        //add msg to list view
        ListViewItem item = new ListViewItem { Text = DateTime.Now.ToString()     };
        item.SubItems.Add(text);
        _listView.Items.Add(ret);
    };
}

You could also extract setup code that attaches ListView to LogMessage into an extension method to make things more DRY.

Side note: DIP is not the same as DI

miensol
  • 39,733
  • 7
  • 116
  • 112
  • If i were to log to the console, i wouldn't ask here for help since it would be that simple, but i want to log to a ListView and that ListView is a part of the Form1. Perhaps you could revise your solution to support an external ListView. –  Jan 02 '16 at 20:50
  • @user1803300 I've updated the code sample. If it still doesn't match your expectations please updated the question accordingly with a sample of how you would like to use the logger inside your custom `Form`. – miensol Jan 02 '16 at 21:00
  • I see your edit, and it looks like you're logging from the Form1 class, i don't want it to look like that, i want a single call to ListViewLogger.Log() and let ListViewLogger handle all that for me. –  Jan 03 '16 at 09:43
  • @user1803300 Now I'm even more confused what's that you want. What's `ListViewLogger.Log()` supposed to do? – miensol Jan 03 '16 at 10:32
  • I just stated what method i want to handle all the logic for me, obviously it will be (inside Form1) called like _lvLogger.Log(). (where _lvLogger is a private readonly field declared as ILogger) –  Jan 03 '16 at 12:23
  • There are 2 things: A: attach ListView to Logger, B: actual logging of messages i.e. Logger.Log("important log messages"); You want the call to `_lvLogger.Log()` to do which of those? – miensol Jan 03 '16 at 12:31
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/99589/discussion-between-miensol-and-user1803300). – miensol Jan 03 '16 at 12:33
  • I want to know how i can pass the ListView reference to the ListViewLogger class without having a method to do it in the ILogger interface, that's what i want –  Jan 03 '16 at 12:33