Final Improvement To ACME's Monolithic Application

Created:11/30/2013 12:14 AM
Updated:12/4/2013 9:48 AM
Source:file:///

Table Of Contents  | <<Previous | Next>>

After six months, the team has produced a version of Wizbang Widget that adheres much more than originally to the SOLID Design Principles and other Principles like Loose Coupling.  The component is much more integrable now, not only into AcquiCorp's product, but into pretty much any product that wants to use it.  At the same time, due to the Deep Synergy Between Testability And Good Design, the Wizbang Component is much more testable.

The important parts of the final code are below, along with a table showing the improvements in this final version of the component.  Compare and contrast the table below with the original table [EDITPOINT: LINK].  Also below are tests  written against the code.  Take a second: peruse the tests and think about how, or if, they could have been written against the original code.  Also look at the code in this final version, and the code in the RuntimeCompositionApplication solution, and think about how the final version of code can integrate with the RuntimeCompositionApplication, compared to the original code.

The code for the final improvement to ACME Corporation's Wizbang Component can be found in the solution Monolithv4.  A couple of things to note about that code:
  • Notice how the Wizbang Component now makes no assumptions whatsoever about what actual implementations it is going to rely on, leaving that up to a client.
  • Wizbang Component does identify interfaces with the contracts it expects from components a client will hook it up to.  The initial impetus was to have Wizbang Component implement the interfaces exposed by AcquiCorp's application, but then the team realized that that would entail a coupling with AcquiCorp's application, reducing the overall integrability of ACME's product and constraining potential for other options - other mergers, etc. - should the AcquiCorp. deal not come through.  By identifying and exposing these interfaces, the team has brought Wizbang Component into adherence with the Interface Segregation Principle, another of the SOLID Design Principles.
  • As a convenience for AcquiCorp, the ACME team has written adapters that will make the Wizbang Component 'plug into' AcquiCorp's product.  Note that these adapters are in their own separate component.  This allows the most flexibility for everyone involved.
And finally, there's another exercise left for you.  ACME's Wizbang Component can be brought even more in line with SOLID Principles and other principles.  Give it a shot: see if you can determine what I might be talking about, or indeed any other improvements regarding SOLID Principles and Loose Coupling, etc. that you can identify.  The next part will identify what I have in mind.  Do yourself a favor: don't peek ahead until you have at least tried to identify this 'post-ultimate' improvement.

For now, though, here's the table I promised, along with the code snippets.  For the full test code, see solution Monolithv4.  Note the design improvements driven by our tests: now in 'DoItAll()', if the Repository returns a null (nonexistent ID), no failure occurs and the appropriate event is logged.  Also, the test that errors are logged showed that an exception thrown by IRepository.GetThing() did not cause a logged event; so the beginning of the try was moved up to accommodate that.  This shows how Unit Tests are a design tool.

Improvements to WizbangComponent
SOLID, DRY, and Loose Coupling Problems
 
Testability Problems
  • No longer has coupling with, or indeed any knowledge of, MonolithFramework or CoolComponent.
  • Very close to full adherence to the Single-Responsibility Principle.  
  • If the requirements change so that it should use anything besides any of these components - for instance, if tomorrow it needs to use UncoolComponent to determine the noise a Thing makes - that can be accomplished much more easily, and specifically it can be accomplished without changing the code.  Thus, in this respect it is adhering to the Open/Closed Principle.
  • It depends on interfaces, not implementations; and specifically on interfaces designed specifically for its needs.  Thus it adheres to the Dependency Inversion Principle ("Depend upon abstractions.  Do not depend on concretions.")
  •  Very easy to test that the appropriate events get logged with the appropriate messages
  • Very easy to test that the appropriate events get broadcast
  • Very easy to test with various responses from the Repository
  • Very easy to test that the class logs appropriately if Thing makes a burp noise
  • Very easy to test that the Noise A Thing Makes gets recorded
  • None of these tests rely on other components 'doing their thing correctly'.  Any bugs in these DOC (Depended-On Components) will not affect tests of the Wizbang Component.
  
    public class WizbangComponent
    {
        private readonly INoiseDeterminer _determiner;
        private readonly INoiseRecorder _recorder;
        private readonly IThingRepository _repository;
        private readonly List< Action< string>> _listeners = new List<Action <string >>();
        private readonly List< Action< string, LogEventType>> _loggingListeners = new List <Action <string ,LogEventType >>();

        public WizbangComponent(
            IThingRepository repository, INoiseDeterminer determiner, INoiseRecorder recorder) {
            _repository = repository;
            _determiner = determiner;
            _recorder = recorder;
        }

        public void AddListener( Action< string> listener) {
            _listeners.Add(listener);
        }

        public void AddLoggingListener( Action< string, LogEventType> loggingListener) {
            _loggingListeners.Add(loggingListener);
        }

        public void DoItAll( string thingId) {
            NoteLoggableEvent( "'DoItAll' was called.", LogEventType .INFO);
            NoteSomeParameterSuitability(thingId);

            try {
                var thing = _repository.GetThing(thingId);

                if (thing == null) {
                    NoteLoggableEvent(
                        "'DoItAll' was called with a thingId ('{0}') which did not refer to an existing Thing.",
                        LogEventType.CONTRACTBREACH);

                    return;
                }

            var doesNotConform = string.Empty;

            if (ThingDoesNotConformToTheRulesWeProvideToAddValue(thing)) {
                NoteLoggableEvent
                    ( string.Format( "Thing '{0}' did not conform to our sophisticated rules,", thing.Name),
                    LogEventType.WARNING);

                doesNotConform = "  But it did not conform to our sophisticated rules.";
            }

                var theNoise = _determiner.DetermineNoise(thing);

                if (theNoise.Contains( "Burp!!!")) {
                    NoteLoggableEvent(
                        string.Format( "Thing {0} made an inappropriate noise." , thing.Name),
                        LogEventType.WARNING);
                }

                _recorder.RecordTheSoundAThingMade(
                    thing, _determiner.WhatIsDoneToTheThing, theNoise);

                NoteLoggableEvent(
                    string.Format(
                        "Thing '{0}' made noise '{1}', and that was recorded.{2}" ,
                        thing.Name, theNoise, doesNotConform),
                    LogEventType.INFO);

                NoteBroadcastableEvent(
                    string.Format( "Thing '{0}' was handled.{1}", thing.Name, doesNotConform));
            }
            catch ( Exception ex) {
                NoteLoggableEvent(
                    string.Format( "ERROR: {0}", ex.Message), LogEventType .ERROR);
        
                throw;
            }
        }

        private bool ThingDoesNotConformToTheRulesWeProvideToAddValue( IThing thing) {
            return thing.Name.ToLower().Contains( "noconform");
        }

        private void NoteSomeParameterSuitability( string thingId) {
            if ( new List< string> { "1", "2" , "noconform" }.Contains(thingId)) {
                NoteBroadcastableEvent( string.Format( "someParameter = '{0}'", thingId));
            }
            else {
                NoteBroadcastableAndLoggableEvent( "someParameter was out of bounds.", LogEventType.CONTRACTBREACH);
            }
        }

        private void NoteBroadcastableEvent( string eventDecription) {
            foreach ( var listener in _listeners) {
                listener(eventDecription);
            }
        }

        private void NoteLoggableEvent(
            string eventDescription, LogEventType logEventType = LogEventType.INFO) {
                foreach ( var logginglistner in _loggingListeners) {
                    logginglistner(eventDescription, logEventType);
                }
        }

        private void NoteBroadcastableAndLoggableEvent(
            string eventDescription, LogEventType logEventType = LogEventType.INFO) {
            NoteBroadcastableEvent(eventDescription);
            NoteLoggableEvent(eventDescription, logEventType);

        }
    }


And here are the tests:
    [TestClass]
    public class WizbangComponentUnitTests {
        [ TestMethod]
        public void WhenDoItAllIsCalledWithSomeParameterNotOutOfBoundsAndReferringToExistingAndConformingThingThenAppropriateLoggableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var loggedEvents = new List< string>();
           
            wizbangComponent.AddLoggingListener(
                (ed, let) => loggedEvents.Add(let + " : " + ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( new IThingToThing(new Thing {Name = "Test Thing" }));

            mockNoiseDeterminer
                .Setup(nd => nd.DetermineNoise( It.IsAny< IThing>()))
                .Returns( string.Empty);

            //Act
            wizbangComponent.DoItAll( "1");

            //Assert
            Assert.AreEqual(
                2, loggedEvents.Count,
                "When DoItAll was called with someParameter not out of bounds, the appropriate Loggable Events were not announced.");
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledWithSomeParameterOutOfBoundsThenAppropriateLoggableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var loggedEvents = new List< string>();

            wizbangComponent.AddLoggingListener(
                (ed, let) => loggedEvents.Add(let + " : " + ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( new IThingToThing(new Thing { Name = "Test Thing" }));

            mockNoiseDeterminer
                .Setup(nd => nd.DetermineNoise( It.IsAny< IThing>()))
                .Returns( string.Empty);

            //Act
            wizbangComponent.DoItAll( "outofbounds");

            //Assert
            Assert.AreEqual(
                3, loggedEvents.Count,
                "When DoItAll was called with someParameter out of bounds, the appropriate Loggable Events were not announced.");
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledWithSomeParameterForNonexistentThingThenAppropriateLoggableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var loggedEvents = new List< string>();

            wizbangComponent.AddLoggingListener(
                (ed, let) => loggedEvents.Add(let + " : " + ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( null as IThing);

            //Act
            wizbangComponent.DoItAll( "1");

            //Assert
            Assert.AreEqual(
                2, loggedEvents.Count,
                "When DoItAll was called with someParameter referring to a nonexistent Thing, the appropriate Loggable Events were not announced.");
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledWithSomeParameterForNonconformingThingThenAppropriateLoggableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var loggedEvents = new List< string>();

            wizbangComponent.AddLoggingListener(
                (ed, let) => loggedEvents.Add(let + " : " + ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( new IThingToThing(new Thing {Name = "noconform" }));

            mockNoiseDeterminer
                .Setup(nd => nd.DetermineNoise( It.IsAny< IThing>()))
                .Returns( string.Empty);

            //Act
            wizbangComponent.DoItAll( "1");

            //Assert
            Assert.AreEqual(
                3, loggedEvents.Count,
                "When DoItAll was called with someParameter fo, the appropriar a nonconforming thing, the appropriate Loggable Events were not announced.");
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledWithSomeParameterNotOutOfBoundsAndReferringToExistingAndConformingThingThenAppropriateBroadcastableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var events = new List< string>();
            wizbangComponent.AddListener(ed => events.Add(ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( new IThingToThing(new Thing { Name = "Test Thing" }));

            mockNoiseDeterminer
                .Setup(nd => nd.DetermineNoise( It.IsAny< IThing>()))
                .Returns( string.Empty);

            //Act
            wizbangComponent.DoItAll( "1");

            //Assert
            Assert.AreEqual(
                2, events.Count,
                "When DoItAll was called with someParameter not out of bounds, the appropriate Broadcastable Events were not announced.");
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledAndExceptionOccursThenAppropriateLoggableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var loggedEvents = new List< string>();
            wizbangComponent.AddLoggingListener(
                (ed, let) => loggedEvents.Add(let + " : " + ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Throws( new Exception( "Fake Exception"));

            //Act
            try {
                wizbangComponent.DoItAll( "1");
            }
            catch {
                //For testing, we'll swallow this exception.
            }
            finally {
                //Assert
                Assert.AreEqual(
                    2, loggedEvents.Count,
                    "When DoItAll was called with someParameter not out of bounds, the appropriate Broadcastable Events were not announced.");
            }
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledAndExceptionOccursThenAppropriateBroadcastableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var events = new List< string>();
            wizbangComponent.AddListener(ed => events.Add(ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Throws( new Exception( "Fake Exception"));

            //mockNoiseDeterminer
            //    .Setup(nd => nd.DetermineNoise(It.IsAny<IThing>()))
            //    .Returns(string.Empty);

            //Act
            try {
                wizbangComponent.DoItAll( "1");
            }
            catch {
                //For testing, we'll swallow this exception.
            }
            finally {
                //Assert
                Assert.AreEqual(
                    1, events.Count,
                    "When DoItAll was called with someParameter not out of bounds, the appropriate Broadcastable Events were not announced.");
            }
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledWithSomeParameterOutOfBoundsThenAppropriateBroadcastableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var events = new List< string>();
            wizbangComponent.AddListener(ed => events.Add(ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( new IThingToThing(new Thing { Name = "Test Thing" }));

            mockNoiseDeterminer
                .Setup(nd => nd.DetermineNoise( It.IsAny< IThing>()))
                .Returns( string.Empty);

            //Act
            wizbangComponent.DoItAll( "outofbounds");

            //Assert
            Assert.AreEqual(
                2, events.Count,
                "When DoItAll was called with someParameter out of bounds, the appropriate Broadcastable Events were not announced.");
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledWithSomeParameterForNonexistentThingThenAppropriateBroadcastableEventMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var events = new List< string>();
            wizbangComponent.AddListener(ed => events.Add(ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( null as IThing);

            //Act
            wizbangComponent.DoItAll( "1");

            //Assert
            Assert.AreEqual(
                1, events.Count,
                "When DoItAll was called with someParameter referring to a nonexistent Thing, the appropriate Broadcastable Events were not announced.");
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledWithSomeParameterForNonconformingThingThenAppropriateBroadcastableEventsMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);

            var events = new List< string>();
            wizbangComponent.AddListener(ed => events.Add(ed));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( new IThingToThing(new Thing { Name = "noconform" }));

            mockNoiseDeterminer
                .Setup(nd => nd.DetermineNoise( It.IsAny< IThing>()))
                .Returns( string.Empty);

            //Act
            wizbangComponent.DoItAll( "1");

            //Assert
            Assert.AreEqual(
                2, events.Count,
                "When DoItAll was called with someParameter foeventsr a nonconforming thing, the appropriate Broadcastable Events were not announced.");
        }

        private struct LoggedEvent {
            internal LogEventType EventType { get ; set ; }
            internal string EventDescription { get; set; }
        }

        [ TestMethod]
        public void WhenDoItAllIsCalledWithAThingIdReferringToAThingThatBurpsThenALoggableEventMustBeAnnounced() {
            //Arrange
            var mockRepository = new Mock< IThingRepository>();
            var mockNoiseDeterminer = new Mock< INoiseDeterminer>();
            var mockNoiseRecorder = new Mock< INoiseRecorder>();

            var wizbangComponent = new WizbangComponent(
                mockRepository.Object, mockNoiseDeterminer.Object, mockNoiseRecorder.Object);
           
            var loggableEvents = new List< LoggedEvent>();

            wizbangComponent.AddLoggingListener(
                (ed, let) => loggableEvents.Add(
                    new LoggedEvent { EventType = let, EventDescription = ed }));

            mockRepository
                .Setup(
                    mr => mr.GetThing( It.IsAny< string>()))
                .Returns( new IThingToThing(new Thing { Name = "1" }));

            mockNoiseDeterminer
                .Setup(nd => nd.DetermineNoise( It.IsAny< IThing>()))
                .Returns( "Burp!!!");

            //Act
            wizbangComponent.DoItAll( "1");

            //Assert
            Assert.IsTrue(
                loggableEvents
                    .Any(
                        le => le.EventType == LogEventType.WARNING
                        &&
                        le.EventDescription.ToLower().Contains( "inappropriate noise")),
                "When DoItAll was called with someParameter for a nonconforming thing, " +
                    "the appropriate Broadcastable Events were not announced." );
        }
    }


<< Previous: Introducing AcquiCorp's Application  |   Next: Bonus >>