How much information hiding is necessary when doing code refactoring?

Question

How much information hiding is necessary? I have boilerplate code before I delete a record, it looks like this:

    public override void OrderProcessing_Delete(Dictionary<string, object> pkColumns)
    {
        var c = Connect();


        using (var cmd = new NpgsqlCommand("SELECT COUNT(*) FROM orders WHERE order_id = :_order_id", c)
            { Parameters = { {"_order_id", pkColumns["order_id"]} } } )
        {
            var count = (long)cmd.ExecuteScalar();

            // deletion's boilerplate code...
            if (count == 0) throw new RecordNotFoundException();
            else if (count > 1) throw new DatabaseStructureChangedException();
            // ...boiler plate code
        }



        // deleting of table(s) goes here...
    }

NOTE: boilerplate code is code-generated, including the "using (var cmd = new NpgsqlCommand( ... )"

But I'm seriously thinking to refactor the boiler plate code, I wanted a more succint code. This is how I envision to refactor the code (made nicer with extension method (not the sole reason ;))

    using (var cmd = new NpgsqlCommand("SELECT COUNT(*) FROM orders WHERE order_id = :_order_id", c)
        { Parameters = { {"_order_id", pkColumns["order_id"]} } } )
    {
              cmd.VerifyDeletion(); // [EDIT: was ExecuteWithVerification before]
    }

I wanted the executescalar and the boilerplate code to goes inside the extension method.

For my code above, does it warrants code refactoring / information hiding? Is my refactored operation looks too opaque?

Answer 1

I would say that your refactor is extremely good, if your new single line of code replaces a handful of lines of code in many places in your program. Especially since the functionality is going to be the same in all of those places.

The programmer coming after you and looking at your code will simply look at the definition of the extension method to find out what it does, and now he knows that this code is defined in one place, so there is no possibility of it differing from place to place.

Answer 2

It's not information-hiding when you extract or refactor your code. It's only information-hiding when you start restricting access to your extension definition after refactoring.

Answer 3

Try it if you must, but my feeling is it's not about succinctness but whether or not you want to enforce the behavior every time or most of the time. And by extension, if the verify-condition changes that it would likely change across the board.

Basically, reducing a small chunk of boiler-plate code doesn't necessarily make things more succinct; it's just one more bit of abstractness the developer has to wade through and understand.

As a developer, I'd have no idea what "ExecuteWithVerify" means. What exactly are we verifying? I'd have to look it up and remember it. But with the boiler-plate code, I can look at the code and understand exactly what's going on.

And by NOT reducing it to a separate method I can also tune the boiler-plate code for cases where exceptions need to be thrown for differing conditions.

Answer 4

"new" operator within a Class (except for the Constructor) should be Avoided at all costs. This is what you need to refactor here.