Just say No! to C# Regions

February 29, 2008

I’ve seen lots of C# code over the last few days, and 100% of it contains the #region construct. So far, I have see it used in only one way: to hide big code blocks. Some of you might be saying “Duh! That’s what they’re for!”.

Well, let’s think about that for a minute:

You need a language construct so that your IDE can help you hide your big messy code block from you (because it’s just so hideous, you don’t even want to look at it anymore)?

If your code is so bad, that you just want to shove it under the covers, then I would argue that your design and the solution to said design is too complex.

Most of the uses I’ve seen of regions are to group member fields, constructors (and related overloads), methods and properties together. This has to be the most retarded way to use a tool because you have other tools to perform this same function. Granted not everyone can buy Resharper, but the File Structure window provides all that same information. Heck, if you simply followed a simple code standard such as:

class <className>
{

  private members
  constructors
  properties
  methods

}

this would do the same job. Honestly, if you can’t LOOK at your code and tell a constructor apart from a property or field, then you should be looking for a different line of work. Please, stop commenting the obvious!

Now, you will eventually get classes that are thousands of lines long, however there are other constructs in the language that can help you keep things neat without using this lame excuse for a preprocessor tag. When you DO find your class encroaching on this large line number threshold, that is a prime time to look at your code for copy/paste refactoring tragedies and do it right. It might be a little painful now, but it could be the thing that saves your project in the long run.

Another lame way I’ve seen regions used is within a method. Usually there is a large block of setup code that is copy and pasted all over the place, with the same region tags. C’mon people, at that point, read about the Extract Method refactoring and actually REUSE that block of code intelligently!

I honestly can’t think of any reason to use #region tags because all the things they are hiding is just really bad code. When you start to just hide bad code, you are less inclined to go back and refactor it properly.

Let’s try this analogy: if you were looking to lose weight before the summer beach season, the best thing to do is get a mirror. This way, you will be re-motivated on a regular basis.

The mere act of having the bad code stare back at you will make sure that it is at least not forgotten. Nearly all codebases have some code that is not koesher. Let’s try to make that lump of code as small as possible. By then, you may even find a way to get rid of that eyesore (and system-sore) all together…after all, it was probably some hack anyway.

Advertisements

Antlr + log4net = ?

February 14, 2008

I’ve been using log4net in nearly every application I’ve written for the past couple of years. It has met my needs in every single instance. From logging to a file, to sending emails, to writing to a console, to doing all three at the same time, you can’t beat log4net (or your own flavor).

I’ve also started to use ANTLR for, what else, parsing some input. I got the hang of parsing input and converting it to an Abstract Syntax Tree without too many issues. However, when it came time to include actions when walking the tree, and finding out the values that are being created or parsed, nothing beats the old printf function. Or rather, WriteLine. However, in all the ANTLR examples I found, everyone was writing to the Console. I don’t like writing to the console directly anymore, as I find that there may be a case when I want to pipe that output to the System Event Log (for example). So, I didn’t think too hard in what needed to come next: a reference to a log4net Logger in the grammar file.

In your grammar file, start with:

@header{

   using System.Reflection;

   using System.Text;        using log4net;

}

Then add a dash of

@members {

   private readonly static ILog Log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); }

Then, in your actions, you can reference your Log object like so:

myRule  : methodName '(' oneParam ')'

{

  Log.Debug("Who cares where I am, I can send my parse output to log4net!");

}

;

Parse with glee now that you can watch the parser do what it does best to your DSLs…

I’ve been a huge fan of Rhino Mocks since I heard about it on a way-back-episode of Dot Net Rocks HanselMinutes. One of the more awesome features of Rhino Mocks is that it utilizes a Record/Playback semantic so that you can set up your mocks with actual calls to the mock objects, then play them back in your tests. This is great if you rely on intellisense when calling your object instances, or if you use Resharper or another code refactoring tool.

Recently, I trying to use Rhino Mocks with DataTables like so:

DataTable a = new DataTable();
DataTable b = new DataTable(); 

MockObject mock = mockRepository.CreateMock<MockObject>();

using (mockRepository.Record())
{
   Expect.Call(mock.CreateRecord("TableName", a, b).Return(1);
}
using (mockRepository.Playback())
{
  MyObject obj = new MyObject(mock);
  obj.Save();
}

When I ran the test, Rhino Mocks says that a the DataTable created during the obj.Save() call wasn’t the same as the one expected. This was correct as there were now two instances, and it seems that the default check Rhino Mocks performs is an Object.Equals(). Since I needed to do some more deep comparisons of the Expected and Actual DataTables, a fellow developer and I started a very simple DataTableConstraint:

    class DataTableConstraint : AbstractConstraint
    {
        private readonly DataTable _expected;
        private string _errorMessage;

        public DataTableConstraint(DataTable expected)
        {
            _expected = expected;
        }

        public override bool Eval(object obj)
        {
            DataTable actual = obj as DataTable;
            if (actual == null)
                return false;

            if (!CheckColumns(actual))
            {
                return false;
            }
            if (!CheckData(actual))
            {
                return false;
            }
            return true;
        }

        private bool CheckData(DataTable actual)
        {
            if (actual.Rows.Count == 0)
            {
                _errorMessage = "Actual table has no data to compare";
                return false;
            }
            try
            {
                for (int i=0; i < _expected.Rows.Count; i++)
                {
                    foreach (DataColumn column in _expected.Columns)
                    {
                        object expectedCell = _expected.Rows[i][column];
                        object actualCell = actual.Rows[i][column];

                        if (expectedCell != actualCell)
                        {
                            _errorMessage = string.Format("Expected {0} in Row ({1}), Column ({2}), but was {3}", expectedCell, i, column.ColumnName, actualCell);
                            return false;
                        }
                    }
                }
                return true;
            }
            catch (System.Exception e)
            {
                _errorMessage = e.Message;
                return false;
            }
        }

        private bool CheckColumns(DataTable actual)
        {
            foreach (DataColumn column in _expected.Columns)
            {
                if (!actual.Columns.Contains(column.ColumnName))
                {
                    _errorMessage = string.Format("Could not find column {0} in {1}", column, actual);
                    return false;
                }
            }
            return true;
        }

        public override string Message
        {
            get { return _errorMessage; }
        }
    }

To use this using the above sample code, do

Expect.Call(mock.CreateRecord(null, null, null).Constraints
(
  Is.Anything(),
  new DataTableConstraint(a),
  new DataTableConstraint(b)
);

Now, this is just a start, and will probably evolve more as I start to get more into verifying the data in two tables. Perhaps Oren will create a new Syntax Helper along the lines of Data.Equals(expectedDataTable) since using a new Constraint() call is a little awkward.