Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a need to write data to file in one of my classes.

Obviously I will pass an interface into my class to decouple it.

I was thinking this interface will be used for testing and also in other projects.

This is my interface:

//This could be used by filesystem, webservice
public interface ILogger
{
   List<string> PreviousLogRecords {get;set;}
   void Log(string Data);
}

public interface IFileLogger : ILogger
{
  string FilePath;
  bool ValidFileName;
}

public class MyClassUnderTest
{
  public MyClassUnderTest(IFileLogger logger) {....}
}

[Test]
public void TestLogger()
{
   var mock = new Mock<IFileLogger>();
   mock.Setup(x => x.Log(Is.Any<string>).AddsDataToList()); //Is this possible??

   var myClass = new MyClassUnderTest(mock.Object);

   myClass.DoSomethingThatWillSplitThisAndLog3Times("1,2,3");

   Assert.AreEqual(3,mock.PreviousLogRecords.Count);

}

This won't work I don't believe as nothing is storing the items so is this possible using Moq and also what do you think of the design of the interface?

share|improve this question
4  
This isn't answering your question, but there are a number of logging frameworks that already exist. These options can do what you need without you having to implement everything. –  unholysampler Mar 21 '12 at 18:57
add comment

1 Answer

up vote 0 down vote accepted

Well, I'm taking a bit of a stab at your first question here, since I'm not clear on what "AddsDataToList" means and your code wouldn't compile either way, but it appears what you're looking to do is use Moq's callback functionality. Something along the lines of:

mock.Setup(x => x.Log(Is.Any<string>()).Callback((string s) => mock.Object.PreviousLogRecords.Add(s));

As for the design of the logger (and apart from the appropriate comment that lots of loggers already exist), it seems reasonable enough, although I personally like to avoid exposing collections as properties. Reason being, the call syntax gets kind of Law-Of-Demeter-violation-ish.

logger.PreviousLogRecords.Count

or

if(logger.PreviousLogRecords[0].Equals("asdf"))

Anyway, on the first count, if you update your post a bit, I can probably comment more specifically on what you're trying to do with Moq.

share|improve this answer
    
I think your Moq code is what I want. Not sure what you mean about Law of Demeter. I guess if I didnt expose a collection the interface could have a AddToList method but not sure how you'd assert that all data has been logged –  Jon Mar 21 '12 at 19:45
    
Law of Demeter is the principle of least knowledge, which can be simplified to say that you should only "talk" with your collaborators. When you have statements like a.b.c.DoX() those are Law of Demeter violations because you have to go picking through a bunch of objects you don't care about to get what you want. This makes code brittle and is best avoided. So, the idea is that as the logger, you should provide clients with what they want -- not with something they have to pick through to get what they want. –  Erik Dietrich Mar 21 '12 at 19:47
    
So you think exposing a ItemsRecordedCount property and n number of string properties to expose n number of previous logs would be nicer? –  Jon Mar 21 '12 at 19:58
    
Well, no, I certainly wouldn't expose n named properties. What I'm talking about here is really a case by case kind of judgment call. I was just suggesting it as something to be aware of. Personally, I'd probably just not bother caching old log messages. Failing that, I'd probably expose a method like GetPreviousLogMessages(int) that returned a collection of the last n log messages. This way, you as the logger control what I get. Your way, as a client of yours, I can call logger.PreviousLogRecords.Clear() and wipe yout your 'internal' cache. –  Erik Dietrich Mar 21 '12 at 20:08
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.