Agile Principles, Patterns, and Practices in C#

When you are finished Clean Code I would recommend you read Bob Martin's other book:

Agile Principles Patterns and Practices In C#

In this book the vast ammount of the book discusses a case study and in it, Bob applies the principles discussed in Clean Code. I read Clean Code first but in retrospect I think "Agile Patterns.." should be read first as Clean Code is more of a day to day handbook or manual of good SW principles.

For example, in "Agile patterns..." the following code is used:

public class OrderData
{
public string customerId;
public int orderId;
public OrderData() {}

...

}

The following validation of the use of Public data deals with your question:

Don't be offended by the use of public data members. This is not an object in the true sense. It is simply a container for data. It has no interesting behavior that needs to be encapsulated. Making the data variables private, and providing getters and setters would be a waste of time. I could have used a struct instead of a class, but I want the OrderData to be passed by reference rather than by value.

Aside:

Personally, I have to say that Robert Martin has made a massive contribution to the SW developer community (along with Martin Fowler, Michael Feathers..) with these books. I think they are must read.

Both patterns delegate to a base class that has several derivative, but it's only in the State pattern that these derivative classes hold a reference back to context class.

Another way to look at it is that the Strategy pattern is a simpler version of the State pattern; a sub-pattern, if you like. It really depends if you want the derived states to hold references back to the context or not (i.e: do you want them to call methods on the context).

For more info: Robert C Martin (& Micah Martin) answer this in their book, "Agile Principles, Patterns and Practices in C#". (http://www.amazon.com/Agile-Principles-Patterns-Practices-C/dp/0131857258)

I strongly suggest not to start restructuring your application without a strong knowledge of SOLID principles and dependency injection. I did this mistake and now I have an application full of service locator (anti)pattern implementations that are not making my life simpler than before.

I suggest you to read at least the following books befor starting:

http://www.amazon.com/Agile-Principles-Patterns-Practices-C/dp/0131857258 (for SOLID principles)

http://www.manning.com/seemann/ (for .NET dependency injection)

http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052

A possible strategy is not refactoring just for the sake of it, but consider refactoring only the parts that are touched more than others. If something is working and nobody is going to change it there's no need to refactor it, it can be a loss of time.

Good luck!

Refactoring HTML is new and relatively good, you can guess what it covers :)

Other than that the two books you mention are the two I've used most, but Agile Principles is also very good.

Your qustion is very subject to opinion...

I love to abstract code and uncouple everywhere possible. The question, as always, is that of time and requirements.

For a small simple project that does not require extensive Unit Testing within the Business Tier, your coupling although does not necessarily follow best practice may be exactly what the customer/end-user requires, and may allow you to provide the software in a more timely fashion.

For a larger/more complex/etc project it may be best to abstract the persistance layer.

It is simply not feasible to follow best practices, best design patterns, and best coding principles for every line of code you write. I've found the authors of such books quite often mention the patterns as potentially surplus to requirements and should simply be used as a tool, when needed.

Hope that helps?

@BryanWatts is right. The classes presented by the OP violate the Liskov Substitution Principle. And you shouldn't use exceptions to control program flow—that's a code smell too. Exceptions are meant for, well, exceptions—exceptional conditions that will not allow your object to behave in an expected manner which could lead to corruption of your object's state and/or future behavior.

You need to ensure that you understand the totality of the Liskov Substitution Principle (LSP). LSP is not about ensuring that interfaces can be used interchangeably.

When an object inherits from another object, it's inheriting all of it's parent's behavior. True, you can override that behavior, but you must be careful in doing so. Let's use your example of a Speaker and a WirelessSpeaker and see how it all falls apart.

public class Speaker
{
    public bool IsPlugged { get; set; }

    public virtual void Beep()
    {
        if (!IsPlugged)
        {
            throw
            new InvalidOperationException("Speaker is not plugged in!");
        }

        Console.WriteLine("Beep.");
    }
}

public class WirelessSpeaker : Speaker
{
    public bool TransmitterIsOn { get; set }

    public override void Beep()
    {
        if (!TransmitterIsOn)
        {
            throw
            new InvalidOperationException("Wireless Speaker transmitter is not on!");
        }

        Console.WriteLine("Beep.");
    }
}

public class IBeepSpeakers
{
    private readonly Speaker _speaker;

    public IBeepSpeakers(Speaker speaker)
    {
        Contract.Requires(speaker != null);
        Contract.Ensures(_speaker != null && _speaker == speaker);
        _speaker = speaker;

        // Since we know we act on speakers, and since we know
        // a speaker needs to be plugged in to beep it, make sure
        // the speaker is plugged in.
        _speaker.IsPlugged = true;
    }

    pubic void BeepTheSpeaker()
    {
        _speaker.Beep();
    }
}

public static class MySpeakerConsoleApp
{
    public static void Main(string[] args)
    {
        BeepWiredSpeaker();

        try
        {
            BeepWirelessSpeaker_Version1();
        }
        catch (InvalidOperationException e)
        {
            Console.WriteLine($"ERROR: e.Message");
        }

        BeepWirelessSpeaker_Version2();
    }

    // We pass in an actual speaker object.
    // This method works as expected.
    public static BeepWiredSpeaker()
    {
        Speaker s = new Speaker();
        IBeepSpeakers wiredSpeakerBeeper = new IBeepSpeakers(s);
        wiredSpeakerBeeper.BeepTheSpeaker();
    }

    public static BeepWirelessSpeaker_Version1()
    {
        // This is a valid assignment.
        Speaker s = new WirelessSpeaker();

        IBeepSpeakers wirelessSpeakerBeeper = new IBeepSpeakers(s);

        // This call will fail!
        // In WirelessSpeaker, we _OVERRODE_ the Beep method to check
        // that TransmitterIsOn is true. But, IBeepSpeakers doesn't
        // know anything _specifically_ about WirelessSpeaker speakers,
        // so it can't set this property!
        // Therefore, an InvalidOperationException will be  thrown.
        wirelessSpeakerBeeper.BeepTheSpeaker();
    }

    public static BeepWirelessSpeaker_Version2()
    {
        Speaker s = new WirelessSpeaker();
        // I'm using a cast, to show here that IBeepSpeakers is really
        // operating on a Speaker object. But, this is one way we can
        // make IBeepSpeakers work, even though it thinks it's dealing
        // only with Speaker objects.
        //
        // Since we set TransmitterIsOn to true, the overridden
        // Beep method will now execute correctly.
        //
        // But, it should be clear that IBeepSpeakers cannot act on both
        // Speakers and WirelessSpeakers in _exactly_ the same way and
        // have confidence that an exception will not be thrown.
        ((WirelessSpeaker)s).TransmitterIsOn = true;

        IBeepSpeakers wirelessSpeakerBeeper = new IBeepSpeaker(s);

        // Beep the speaker. This will work because TransmitterIsOn is true.
        wirelessSpeakerBeeper.BeepTheSpeaker();
}

This is how your code broke the Liskov Substitution Principle (LSP). As Robert & Micah Martin astutely point out in Agile Principles, Patterns and Practices in C# on pp. 142-143:

LSP makes it clear that in OOD, the IS-A relationship pertains to behavior that can be reasonably assumed and that clients depend on....[W]hen using an object through its base class interface, the user knows only the preconditions and postconditions of the base class. Thus, derived objects must not expect such users to obey preconditions that are stronger than those required by the base class. That is, users must accept anything that the base class could accept. Also, derived classes must conform to all the postconditions of the base [class].

By essentially having the precondition TransmitterIsOn == true for the Beep method of the WirelessSpeaker, you created a stronger precondition than what existed on the base Speaker class. For WirelessSpeakers, both IsPlugged and TransmitterIsOn must be true in order for Beep to behave as expected (when viewed from the perspective of a Speaker), even though a Speaker in and of itself has no notion of TransmitterIsOn.

Also, you violated another SOLID principle, the Interface Segregation Principle (ISP):

Clients should not be forced to depend on methods they do not use.

In this case, a WirelessSpeaker does not need to be plugged in. (I'm assuming we're talking about the audio input connection here, as opposed to an electrical connection.) Therefore, the WirelessSpeaker should not have any property called IsPlugged, yet, because it inherits from Speaker, it does! This is an indication that your object model does not line up with how you intend to use your objects. Again, notice that most of this discussion is centered around the behavior of your objects, and not their relationship to one another.

Moreover, the violation of both LSP and ISP both signal that there's probably been a violation of the Open/Closed Principle (OCP), too:

Software entities (classes, modules, functions, etc.) should be open for extension but closed for modification.

So, at this point it should be clear, now, that we do not use Code Contracts only to ensure that certain preconditions are met when calling methods on objects. No, instead Code Contracts are used to state guarantees (hence the word contract) about the behavior and state of your objects and their methods based on the stated pre- and post-conditions, as well as any invariants you may also have defined.

So, for your speaker class, what you're saying is: If the speaker is plugged in, then the speaker can beep. Ok, so far, so good; that's simple enough. Now, what about the WirelessSpeaker class?

Well, WirelessSpeaker inherits from Speaker. Therefore, WirelessSpeaker also has an IsPlugged Boolean property. Furthermore, because it inherits from Speaker, then in order for the WirelessSpeaker to beep, it must also have its IsPlugged property set to true. "But wait!" you say, "I have overridden the implementation of Beep such that WirelessSpeaker's transmitter must be on." Yes, this is true. But it also must be plugged in! WirelessSpeaker not only inherits the Beep method, but also the behavior of its parent class's implementation! (Consider when a base class reference is used in place of the derived class.) Since the parent class can be "plugged in", so too, can WirelessSpeaker; I doubt this is what you intended when you originally thought of this object hierarchy.

So, how would you fix this? Well, you need to come up with a model better aligned to the behaviors of the objects in question. What do we know about these objects, and their behavior?

1. They're both a kind of speaker.
   • So potentially, a wireless speaker could be a specialization of a speaker. Conversely, a speaker may be a generalization of a wireless speaker.
   • In the current object model (as much as you've posted), there is not much behavior or state that is shared between these two objects.
   • Since there is not much common state or behavior between the two objects, one could argue there shouldn't be an inheritance hierarchy here. I'm going to play devil's advocate with you and maintain the inhertiance hierarchy.
2. They both beep.
   • However, the conditions under which each type of speaker beeps are different.
   • These speakers, therefore, cannot directly inherit one from the other, otherwise, they would share behavior which may not be appropriate to them (and in case, the existing "shared behavior" is definitely not appropriate for all types of speakers). This resolves the ISP problem.

Ok, so the one piece of shared behavior these speakers have is they beep. So, let's abstract that behavior out into an abstract base class:

// NOTE: I would prefer to simply call this Speaker, and call
// Speaker 'WiredSpeaker' instead--but to leave your concrete class
// names as they were in your original code, I've chosen to call this
// SpeakerBase.
public abstract class SpeakerBase
{
    protected SpeakerBase() { }

    public void Beep()
    {
        if (CanBeep())
        {
            Console.WriteLine("Beep.");
        }
    }

    public abstract bool CanBeep();
}

Great! Now we have an abstract base class that represents speakers. This abstract class will allow a speaker to beep, if and only if the CanBeep() method returns true. And this method is abstract, so any class inheriting this class must provide their own logic for this method. By creating this abstract base class, we have enabled any class that has a dependency on the SpeakerBase class to emit a beep from the speaker if and only if CanBeep() returns true. This also resolves the LSP violation! Anywhere a SpeakerBase can be used and called upon to beep, either a Speaker or a WirelessSpeaker can be substituted and we can be sure of the behavior: if the speaker can beep, it will.

Now all that's left is to derive each of our speaker types from SpeakerBase:

public class Speaker : SpeakerBase
{
    public bool IsPlugged { get; set; }

    public override bool CanBeep() => IsPlugged;
}

public class WirelessSpeaker : SpeakerBase
{
    public bool IsTransmiterOn { get; set; }

    public override bool CanBeep() => IsTransmitterOn;
}

So, now we have a Speaker that can only beep when it's plugged in. We also have a WirelessSpeaker that can only beep if it's transmitter is turned on. In addition, WirelessSpeakers know nothing about being "plugged in". It's simply not a part of their essence.

Furthermore, following the Dependency Inversion Principle (DIP):

1. High-level modules should not depend on low-level modules. Both should depend on abstractions.
2. Abstractions should not depend upon details. Details should depend upon abstractions.

What this means is that consumers of speakers should not depend directly on either Speaker or WirelessSpeaker, but should depend on SpeakerBase instead. This way, no matter what kind of speaker comes along, if it inherits from SpeakerBase, we know that we can beep the speaker if the conditions warrant for the sub-type of speaker referenced by the abstract type in the dependent class. This also means that IBeepSpeakers no longer knows how to put a speaker in the state such that it can beep, as there is no common behavior among speaker types that IBeepSpeakers could use to make such a determination. So that behavior must be passed in as a dependency to IBeepSpeakers. (This is an optional dependency; you could just let the class take in a SpeakerBase and call Beep(), and, if the SpeakerBase object is in the correct state, it'll beep, otherwise it won't.)

public class IBeepSpeakers
{
    private readonly SpeakerBase _speaker;
    private readonly Action<SpeakerBase> _enableBeeping;

    public IBeepSpeakers(SpeakerBase speaker, Action<SpeakerBase> enableBeeping)
    {
        Contract.Requires(speaker != null);
        Contract.Requires(enableBeeping != null);
        Contract.Ensures(
            _speaker != null && 
            _speaker == speaker);
        Contract.Ensures(
            _enableBeeping != null && 
            _enableBeeping == enableBeeping);

        _speaker = speaker;
        _enableBeeping = enableBeeping;
    }

    pubic void BeepTheSpeaker()
    {
        if (!_speaker.CanBeep())
        {
           _enableBeeping(_speaker);
        }
        _speaker.Beep();
    }
}

public static class MySpeakerConsoleApp
{
    public static void Main(string[] args)
    {
        BeepWiredSpeaker();

        // No more try...catch needed. This can't possibly fail!
        BeepWirelessSpeaker();
    }

    public static BeepWiredSpeaker()
    {
        Speaker s = new Speaker();
        IBeepSpeakers wiredSpeakerBeeper =
            new IBeepSpeakers(s, s => ((Speaker)s).IsPlugged = true);
        wiredSpeakerBeeper.BeepTheSpeaker();
    }

    public static BeepWirelessSpeaker()
    {
        WirelessSpeaker w = new WirelessSpeaker();
        IBeepSpeakers wirelessSpeakerBeeper =
            new IBeepSpeakers(w, s => ((WiredSpeaker)s).IsTransmitterOn = true);
        wirelessSpeakerBeeper.BeepTheSpeaker();
    }
}

As you can see, we actually didn't need Code Contracts at all to tell us whether or not the speaker should beep. No, rather we let the state of the object itself determine whether it can beep.

In my opinion, Null is preferable over throwing an Exception, but that depends on how exceptional this case is, and of course your domain.

A nicer thing to return is a Null object, i.e. an Object that implements the same Interface as the item you are looking up in your repository, but it has behaviour that makes it nicer to work with than just null.

Read up on the Null object pattern, or look it up in Agile Principles, Patterns, and Practices in C#

Based on your examples, there's definitely an ISP and a SRP and probably a Law of Demeter (not SOLID but...) violation going on at the very least.

IMNSHO You're far better off reading articles on SOLID (or buying Agile Principles, Patterns, and Practices in C# by Robert C. Martin and Micah Martin which is excellent throughout and one of the most useful books I've read in recent years) than asking for piecemeal advice on the interwebs for this type of thing.

If you want a shortcut (you don't though - the books and PDFs have examples that explain things very well!), these Hanselminutes podcasts with Uncle Bob are very good:

• Hanselminutes Podcast 145 - SOLID Principles with Uncle Bob - Robert C. Martin
• Hanselminutes Podcast 171 - The Return of Uncle Bob

edit: Nabbed the SRP from Jon Limjap and ppiotrowicz

This is certainly a reasonable explanation:

http://en.wikipedia.org/wiki/Inversion_of_control

I own this book: I'd recommend it for good coverage of many design principles, including IoC:

Agile Principles, Patterns and Practices in C#: Martin Fowler, Micah Fowler

You might also find these links helpful:

• DI and IOC examples in simple C#

• Inversion of Control < Dependency Injection

Two pithy "explanations":

• IoC is sometimes known as the "Hollywood Principle": Don't call us, we'll call you

• IoC = Marriage; IOC Container = Wife

This sounds an integration test to me. Anyways, forget RhinoMock in this case because there's no other way to do it but to create your own test suite here. In our case we actually used HttpClient to call the controller/api and pass the url and argument needed for action and anticipate the result for validation.

public class ClientHandler
{
   private HttpClient _client;

   public ClientHander()
   {
        // add handler if needed ex var handler = new HttpClientHandler()

        _client = new HttpClient(/*handler*/);

        // add default header if needed client.DefaultRequestHeaders
   }

   public HttpResponseMessage Post(string path, HttpContent content)
   {
      // You can async if you want
      return _client.Post(path, content).Result;
   }
}

Now you can use it in your actual testing.

[TestClass]
public class MyController
{
    [TestMethod]
    public void TestMyControllerActionSaveData()
    {  
         var client = new ClientHandler();
         var content = new StringContent(dataHere, Encoding.UTF8, "application/json");
         var outPut = client.Post("http://server/action/here", content).Result;

         // validate expected output here
    }
}

There are a lot of missing configuration and setup here but the gist is there.

Update : I really like what you are currently doing in the name of testing because this is a powerful tool as part of Continues Integration (CI). Just a minor comment the way to name your test method. I would suggest to rename the test method into an action you want to do and put those procedure inside the test like what Gherkin way of BDD or as described by Dan North .

[TestMethod]
public void Should_Save_If_Has_Data()
{
   Given(Web_Service_Instance)
     With(Data_To_Pass)
     When(Posting_Data_To_Service)
     Then(Data_Will_Be_Saved)
     Verify()
}

[TestMethod]
public void Should_Not_Save_If_No_Data()
{
    .....
}

If you can create a TestSuite like described above it will give you a lot of benefits in the long run and avoid repetition of code (DRY). Also those tests will serve as living documents as described by Robert C. Martin & Micah Martin. Special thanks to the team that Im involved and kudos is for them!

I advise you to take note of a book Agile Principles, Patterns, and Practices in C# by Robert C. Martin and Micah Martin, there are many good examples where it is shown how to design a system by UML, and other similar methods. Specifically, it is shown why you should refactor your code, that can be stored in abstract classes and what is not. You can immediately see an example with a coffee maker Mark IV, where he developed a truly independent interface.

According to my feelings, the main principle of MVVM - is the independence of the data from its representations. I like trying to make separate modules, which implement separate logic, not knowing about the data. For example, SQL module, SMTP module, etc, which simply contain some methods like ConnectToDb() or SendEmail(), the main logic is in ViewModel, she combines these Work-modules with the data model.

Useful to draw a diagram before designing the system, but do not get involved too. The main thing is to see the main parts in the paper, and that the others would understand it as well as you know (or architect).

First of all, it is this book:

http://www.amazon.com/Agile-Principles-Patterns-Practices-C/dp/0131857258/ref=sr_1_5?ie=UTF8&qid=1308502374&sr=8-5

Robert Martin - it is a person who introduced SOLID principles.

You will want to learn more about the entire Agile Development movement. Agile is just what it says: able to quickly adapt.

• Robert C. Martin's 2002 book - a classic.

• Agile in C# - this is what I use

• See this list of books

• You'll want to read the Agile Manifesto!

• Just one more: Kent Beck's work - especially Test-Driven Development.