Archives

Pragmatic SOLID Part 1 – The Single Responsibility Principle

There should never be more than one reason for a class to change. – Robert C. Martin February 1997

TL;DR

  • Break up your classes into teeny tiny ones.
  • You’re aiming to limit the impact and complexity of change within a class.
  • Don’t use brainlessly – there needs to be a symptom or good reason.

What’s the business value?

The ideal outcome of applying the Single Responsibility Principle (SRP) is to reduce assessment, development and testing time when making changes to a class in future. We like that, right? We could even use that in a discussion with Product Owners around reducing technical debt! Wait, it does more than that. The principle consequently effects an improved flexibility and reuse pattern through a class which is simpler, smaller and less prone to consequential defects. Each resulting class starts to become more cohesive and testable.

This is achieved by decomposing your (lovely) encapsulated class by separating concerns within the class, into smaller classes.

A class’ data, calculation capability, save/load capability and print capability result in four classes. You then have ultimate confidence what the impact of your change will be. Some would even go so far as to split save from load.

It’s not all good news though. Some people quote the SRP to say “this class does one thing and one thing only, really well”, I think that’s a misinterpretation. Applying the principle unnecessarily can quickly lead to increased complexity and confusion on large projects due to high volumes of small, poorly named single-method classes which would have previously been encapsulated within one class.  Good naming conventions, and feature grouping can help with this, but why are you now fighting with your code? Something went wrong.

Show me an example!

Here’s an example which demonstrates the SRP applied to a class which encapsulates a graph. The points are calculated based on a transformation model. The graph can persist and restore its settings on disk, and provides the capability to print given a Printer class.

namespace MyGraphProgramV1
{
    // dummy class
    class Printer
    {
    }

    static class Program
    {
        // small, fictional program to load up a previous graph settings file
        // print the old version, update the title and mathematical model
        // and resave the settings
        static void Main()
        {

            MyGraph graph = new MyGraph();

            graph.Load(@"C:\settings.txt");

            Printer printer = new Printer();
            graph.Print(printer);

            graph.GraphTitle = "NewTitle";
            graph.MathModel = TransformationModel.modelX2;

            graph.Save(@"C:\settings.txt");
        }
    }

    public enum TransformationModel { none, modelX1, modelX2, modelX3 };

    class MyGraph
    {
        // version 1 - multiple reasons to change
        // 1. If a setting changes
        // 2. if the printing method changes
        // 3. If the calculation engine changes
        // 4. If the storage mechanics change
        // 5. If the point array changes
        // the class as a whole is at risk when each of these change. 
        // imagine if each function was a few hundred lines. 
        // This would start to become difficult to read.

        public MyGraph() { ; }

        public String GraphTitle
        {
            set
            {
                graphTitle = value;
            }
            get
            {
                return graphTitle;
            }
        }

        public TransformationModel MathModel
        {
            get { return transformationModel; }
            set
            {
                transformationModel = value;
                CalculatePoints();
            }
        }

        public void Load(String fileName)
        {
            // load title and transformation model, probably from an xml file
            // possibly upgrade the settings in future.
            CalculatePoints();
        }

        public void Save(String fileName)
        {
            // save title and transformation model, handle file exceptions. 
        }

        public void Print(Printer printer)
        {
            // print title and points, draw some axis, make it look pretty.
            // this function could get quite large and use a lot of common classes.
        }

        private void CalculatePoints()
        {
            // depending on transformationModel, 
            // populate points with a mathematical algorithm
            // this will probably be a large switch statement to start with.
        }

        private int[,] points = new int[10, 10];
        private TransformationModel transformationModel;
        private String graphTitle;
    }
}

As you will see with the Single Responsibility Principle applied below, some classes still have more
than one reason for change. You can chase down the rabbit hole but that can get silly. You have to ask yourself where do you require reduced complexity, clarity, reuse, testability. The code below reduces the impact of a change on the entirety of the MyGraph class.

namespace MyGraphProgramV2
{
    // dummy class
    class Printer
    {
    }

    static class Program
    {
        // small, fictional program to load up a previous graph settings file
        // print the old version, update the title and mathematical model
        // and re-save the settings
        static void Main()
        {
            // version 2 Single Responsibility Principle implemented 
            // Note: there are no changes to our client!.
            MyGraph graph = new MyGraph();

            graph.Load(@"C:\settings.txt");

            Printer printer = new Printer();
            graph.Print(printer);

            graph.GraphTitle = "NewTitle";
            graph.MathModel = TransformationModel.modelX2;

            graph.Save(@"C:\settings.txt");
        }
    }

    public enum TransformationModel {none, modelX1, modelX2, modelX3};

    class MyGraph
    {
        // version 2 - Single responsibility principle applied
        // This class' responsibility is now to orchestrate and 
        // route data between SRP classes.
        // It's much easier to read. Reasons for change:
        //    1. If classes require more data or different interfaces
        //    2. If we add more properties we have to route them on to the settings.

        // We could create these classes when needed, most don't need to be members. #todo
        private MyGraphSettingsModel settings = new MyGraphSettingsModel();
        private MyGraphDataModel dataModel = new MyGraphDataModel();
        private MyGraphStorage storage = null;
        private MyGraphPrinter printer = null;
        private MyGraphPointsCalculator calc = null;

        public MyGraph()
        {
            storage = new MyGraphStorage(settings);
            printer = new MyGraphPrinter(dataModel);
            calc = new MyGraphPointsCalculator(dataModel);
        }

        public TransformationModel MathModel
        {
            get 
            { 
                return settings.Model; 
            }
            set
            {
                settings.Model = value;
                calc.CalculatePoints(settings.Model);
            }
        }

        public String GraphTitle 
        {
            get { return settings.GraphTitle; }
            set { settings.GraphTitle = value; }
        }

        public void Load(String fileName)
        {
            storage.Load(fileName);
            calc.CalculatePoints(settings.Model); // need to calculate points after a load
        }

        public void Save(String fileName)
        {
            storage.Save(fileName);
        }

        public void Print(Printer printerDevice)
        {
            printer.Print(printerDevice);
        }
    }

    class MyGraphSettingsModel
    {
        // This class' reasons for change: 
        // 1. If a property is added or removed from the settings class

        private TransformationModel model;
        private String graphTitle;

        public TransformationModel Model
        {
            get { return model; }
            set { model = value; }
        }

        public String GraphTitle
        {
            get { return graphTitle; }
            set { graphTitle = value; }
        }
    }

    class MyGraphDataModel
    {
        // This class' reasons for change: 
        //    1. If the data points model changes 

        public int[,] points = new int[10,10];
    }

    class MyGraphStorage
    {
        // This class' reasons for change:
        //  1. if the settings model changes 
        //     - this class could get busy if we need to perform 
        //       upgrades on older versions when loading.
        //     - by its own merit that's a great reason to move 
        //       it away from the main class
        //  2. If we change from file storage to internet/memory 
        //     - so we could separate further through interfaces.
        // 
       private MyGraphSettingsModel settings = null;

       public MyGraphStorage(MyGraphSettingsModel model)
       {
           settings = model;
       }

       public void Save(String fileName) { return; } // does nothing for now
       public void Load(String fileName) { return; } // does nothing for now
    }

    class MyGraphPrinter
    {
        // This class' reasons for change
        // 1. If the data model changes, we have to update the 
        //    printing routine - we could pass explicit properties instead
        // 2. if the printer API/interface changes
        private MyGraphDataModel dataModel = null;
        public MyGraphPrinter(MyGraphDataModel model)
        {
            dataModel = model;
        }
        public void Print(Printer printer) { }
    }

    class MyGraphPointsCalculator
    {
        // This class' reasons for change
        // 1. The implementation of the calculation engine changes
        // 2. a new calculation is required.
        MyGraphDataModel dataModel = null;
        public MyGraphPointsCalculator(MyGraphDataModel model) 
        { dataModel = model; }
        public void CalculatePoints(TransformationModel model) 
        { ; } // update points
    }
}

When should I retrospectively apply the principle to a class?

In Robert Martin’s Book (Agile principles, patterns and practices in C#), he warns against applying the Single Responsibility Principle to introduce needless complexity. Ask yourself if you have skim read over that line. At times, I certainly forget that!

The Middleton test – to maximise ROI:

  • If unexpected defects are regularly introduced when this class is altered, decouple concerns, apply the principle to the class.
  • If you are planning on duplicating part of the functionality of this class, apply the principle to that part.
  • If you require future flexibility for varying implementations (E.g. calculation engines, drawing routines, printing routines, saving to different media), then apply the principle in anticipation of this.
  • If none of the above are true, and you have more valuable things to be undertaking – do them instead.

When exercising the principle, your newly extracted classes are ripe candidates for a strong set of supporting tests. This is especially important if the extracted classes are still complex internally and you plan to reuse it.

As you can tell I’m not an advocate of hyper-proactive application of SRP. I’m not alone. My bold assertion is – that If you are confident that your class is not going to be subject to change, doesn’t exhibit a set of bugs which flip-flop regularly and its code offers no future re-use, then it probably isn’t cost effective to apply the SRP to it.

So what is the official description of the Single Responsibility Principle?

Robert C Martin (Uncle Bob). Feb 1997
“In the context of the Single Responsibility Principle (SRP) we define a responsibility to be “a reason for change.” If you can think of more than one motive for changing a class, then that class has more than one responsibility.”
“If a class has more then one responsibility, then the responsibilities become coupled. Changes to one responsibility may impair or inhibit the class’ ability to meet the others. This kind of coupling leads to fragile designs that break in unexpected ways when changed”

The typical examples given across the web are:

  • a class that not only holds the document data, but saves and prints it too.
  • a rectangle (or other geometric) class that has an area calculation function and a drawing function.

References

Full description (Chapter Excerpt) from Robert C. Martin

Developer Express video on SRP

Wikipedia

Curly’s Law – Jeff Atwood

OODesign.com

Bob Martin explaining the SRP

And, just google. There’s lots of great examples and viewpoints.

(C) Giles Middleton  2013

Initializing MFC/CRT for consumption of regional settings (Internationalization/C++)

Summary

In this post I’m going to demonstrate how to make sure C++ programs use the regional settings correctly when using Microsoft Foundation Classes. You think it’s a fully transparent process and already done for you? Not always! I’m going to start with the conclusion, and you can read more if you need to. I hope people find this useful.

Conclusion

Put the following code in a function and call it from your Application’s initialization routine, or you may get incomplete functionality within MFC classes which mix use of CRT and other DLLs to format numbers and dates.

setlocale(LC_COLLATE,“.OCP”); // sets the sort order

setlocale(LC_MONETARY, “.OCP”); // sets the currency formatting rules

setlocale(LC_NUMERIC, “.OCP”); // sets the formatting of numerals

setlocale(LC_TIME, “.OCP”); // defines the date/time formatting

Background

I’m going to walk you through how I arrived at the conclusion above, supported by some code samples and other articles.

Whilst reviewing some region-aware code in C++, I noticed some differences to the way in which C++ handles it versus C#. In C# this is (relatively) easy, as the majority of the .NET classes are geared up for Culture awareness. In MFC, it (at first) appeared to be very similar, although hardly any classes provided the overloads you get in C# .NET.

During automated testing of the code, strange results caused me hours of head scratching. The worse thing a programmer can face is inconsistent results. We like things to fail or pass 100% of the time.

For the purposes of this example, I created an MFC application in Visual Studio 2010, using a dialog based application type with no document/view support. My code is placed into the OnAppAbout event handler (provided as default) as a playpen area.

Here I’m simply tracing the ‘localized’ time format, and then specifying a more verbose time format that includes the abbreviated month and day names.

void CInternationalizationExampleApp::OnAppAbout()

{

COleDateTime tm(2001,5,1,12,13,14);

TRACE( tm.Format()+“\n” + tm.Format(L“%Y/%m (%b)/%d (%a) %X”));

}

This results in the following strings (i’m UK locale, so I expect day/month/year):

01/05/2001 12:13:14
2001/05 (May)/01 (Tue) 12:13:14

Now, if I change the regional settings to Swedish (Sweden) in the Region and Language Options dialog, press Apply, and rerun the code – I expect to see the outputs change accordingly.

Changing the regional settings temporarily.

Changing the regional settings temporarily.

In Swedish, May is Maj, and Tue is Ti. Their short date format is also ISO style, with year, month and date separated with hyphens.

Results:

2001-05-01 12:13:14
2001/05 (May)/01 (Tue) 12:13:14

COleDateTime has used the correct regional settings for .Format(), but not for Format( string ). On closer inspection of the two functions we can see subtle differences : – notice the code highlighted.

inline CString COleDateTime::Format(_In_ DWORD dwFlags,_In_ LCID lcid) const

{

// If null, return empty string

if (GetStatus() == null)

return _T(“”);

// If invalid, return DateTime global string

if (GetStatus() == invalid)

{

CString str;

if(str.LoadString(ATL_IDS_DATETIME_INVALID))

return str;

return szInvalidDateTime;

}

CComBSTR bstr;

if (FAILED(::VarBstrFromDate(m_dt, lcid, dwFlags, &bstr)))

{

CString str;

if(str.LoadString(ATL_IDS_DATETIME_INVALID))

return str;

return szInvalidDateTime;

}

CString tmp = CString(bstr);

return tmp;

}

and

inline CString COleDateTime::Format(_In_z_ LPCTSTR pFormat) const

{

ATLENSURE_THROW(pFormat != NULL, E_INVALIDARG);

// If null, return empty string

if(GetStatus() == null)

return _T(“”);

// If invalid, return DateTime global string

if(GetStatus() == invalid)

{

CString str;

if(str.LoadString(ATL_IDS_DATETIME_INVALID))

return str;

return szInvalidDateTime;

}

UDATE ud;

if (S_OK != VarUdateFromDate(m_dt, 0, &ud))

{

CString str;

if(str.LoadString(ATL_IDS_DATETIME_INVALID))

return str;

return szInvalidDateTime;

}

struct tm tmTemp;

tmTemp.tm_sec = ud.st.wSecond;

tmTemp.tm_min = ud.st.wMinute;

tmTemp.tm_hour = ud.st.wHour;

tmTemp.tm_mday = ud.st.wDay;

tmTemp.tm_mon = ud.st.wMonth – 1;

tmTemp.tm_year = ud.st.wYear – 1900;

tmTemp.tm_wday = ud.st.wDayOfWeek;

tmTemp.tm_yday = ud.wDayOfYear – 1;

tmTemp.tm_isdst = 0;

CString strDate;

LPTSTR lpszTemp = strDate.GetBufferSetLength(256);

_tcsftime(lpszTemp, strDate.GetLength(), pFormat, &tmTemp);

strDate.ReleaseBuffer();

return strDate;

}

On stepping into the code and checking on MSDN, the VarBstrFromDate function lives in oleaut32.dll. The code trail goes cold in a non debug windows environment at this point. Whereas _tcsftime is in msvcr100d.dll, which we can step into and we see references to the _locale_t data type and a set of localization functions which have similar signatures e.g. suffix with _l (_tcsftime_l).

So the CRT is supposed to be locale aware under the hood. This told me either two different mechanisms are at play or some sort of initialization is required (most probable).

On further googling, I confirmed that the functions which interact with the regional settings are in Kernel32.dll, and are called things like GetLocaleInfo()(see WinNLS.h). Whereas CRT uses functions like setlocale and suffixes functions with with _l to allow a _locale_t object to be passed. By design, it has similarly named functions such as __getlocaleinfo.

I found the following articles on CodeProject that further confirmed my suspicions.

http://www.codeproject.com/Articles/9600/Windows-SetThreadLocale-and-CRT-setlocale

http://www.codeproject.com/Articles/12568/Format-Date-and-Time-As-Per-User-s-Locale-Settings

We can test this theory by setting the locale to Swedish using the setlocale function.

#include <locale.h>

void CInternationalizationExampleApp::OnAppAbout()

{

COleDateTime tm(2001,5,1,12,13,14);

setlocale( LC_TIME, “swedish” );

TRACE( tm.Format()+“\n”+tm.Format(L“%Y/%m (%b)/%d (%a) %X”));

}

Results in:

2001-05-01 12:13:14
2001/05 (maj)/01 (ti) 12:13:14

This is good news, but I need it to work based off the current user profile by default and I don’t want to have the method described in the code project articles if I can help it. I’m not switching locales.

On a closer inspection of the CRT source code, I traced down what happens in setlocale() and it calls __getlocaleinfo, __crtGetLocaleInfoA and the function that really matters: __crtGetLocaleInfoA_stat – it calls GetLocaleInfoW from Kernel32.dll. Bingo.

If I set a breakpoint in __crtGetLocaleInfoA_stat it doesn’t fire unlesswe call setlocale().

This indicates that the CRT does not populate it’s locale buffers when it starts, nor lazily, but that it has to be explicitly set (as the code project articles correctly state).

Looking at the setlocale MSDN article we can see there’s a shortcut to initialize CRT based on the current regional settings.

setlocale( LC_ALL, ".OCP" );

Explicitly sets the locale to the current OEM code page obtained from the operating system.

I can successfully put this code in, and my test code has the same (correct) result

void CInternationalizationExampleApp::OnAppAbout()

{

COleDateTime tm(2001,5,1,12,13,14);

setlocale( LC_ALL, “.OCP” );

//setlocale( LC_TIME, “swedish” );

TRACE( tm.Format()+“\n” + tm.Format(L“%Y/%m (%b)/%d (%a) %X”));

}

Results:

2001-05-01 12:13:14
2001/05 (maj)/01 (ti) 12:13:14

In the article pasted below, Microsoft recommends NOT using LC_ALL or LC_CTYPE. http://msdn.microsoft.com/en-us/goglobal/bb688121.aspx

Note: The code from the article doesn’t actually compile, and you can use setlocale() and get rid of the Unicode L and explicit use of _w.

C Run Time

CRT locale support is built around the (_w) setlocale (category, locale) call. A call to this function defines the results of all subsequent CRT-based locale-sensitive operations, not only the character encoding. The category argument defines scope of environment changes after setlocale is called.

In order to set the rules for formatting locale-sensitive data in accordance with the user locale, the following calls can be executed:

_wsetlocale (LC_COLLATE, L(".OCP") ); // sets the sort order
_wsetlocale (LC_MONETARY, L(".OCP") ); // sets the currency formatting rules
_wsetlocale (LC_NUMERIC, L(".OCP") ); // sets the formatting of numerals
_wsetlocale (LC_TIME, L(".OCP") ); // defines the date/time formatting

“.OCP” and “.ACP” parameters always refer to the settings of the user locale, not the system locale. While selecting this locale for LC_CTYPE or LC_ALL is not a good choice, all other categories should be set to match the user locale, unless your console must be explicitly independent of the user’s settings.