Tuesday 26 November 2013

if errorlevel 1 vs if %errorlevel% neq 0

The problem with having used batch files since the days of DOS is that certain constructs are just way too embedded in your brain. For example the way I check whether a command in a batch file has failed is by using the special “errorlevel” version of the “if” statement:-

ExternalProgram.exe
if errorlevel 1 exit /b 1

This style of “if” says that if the exit code from the last run program run was greater than or equal to 1 the script should terminate, with an exit code of 1. The general convention for exit codes is that 0 means success and anything else just means “something else happened” [1] and it’s entirely dependent on the program. For example the ROBOCOPY utility has a variety of different codes that may or may not be an error depending on what you asked it to do. The MSI installer (MSIEXEC.EXE) is another program that has a few exit codes that you soon get to know if you’re trying to script a deployment process, e.g.

msiexec /x <GUID>
if %errorlevel% equ 1605 goto :not_installed

This form of “if” statement (with a variable called errorlevel) is the newer form that was introduced in Windows NT (I think) and it allows you to do an equality comparison with a single exit code, which was less than intuitive before. This form is also required when you have anti-social processes that return negative exit codes [2]. In fact the earlier form should probably be considered defunct (if only my muscle memory would let go) and the newer form used by default instead:-

ExternalProgram.exe
if %errorlevel% neq 0 exit /b %errorlevel%

If you can’t remember what the operators are use “C:\> HELP IF” to list them [3].

[1] C & C++ programmers will of course already be used to using the macros EXIT_SUCCESS and EXIT_FAILURE from <stdlib.h>. I don’t think .Net has an equivalent and so I often end up creating a class called ExitCode with the same two constants.

[2] Yes SCHTASKS (the command line tool for managing scheduled tasks) I’m looking at you. The .Net runtime can also chuck out negative exit codes if something really serious goes wrong, e.g. the process fails to start with an out of memory or assembly loading problem.

[3] They’re different from PowerShell too which is a shame.

Logging Stack Traces Should be Unnecessary

I like a nice clean log file. The left hand margin should be a fixed width and easy on the eye so that my built-in pattern recognition gets to work as effectively as possible. It should also be easy to parse with the age old UNIX command line tools (and LogParser) without me having to pre-process the file first to get shot of the noise.

What really messes with this is when I see stack traces in the log file. They are always huge and contain far too much duplication because modern software design principles suggest we write short, simple methods, with overloads and reuse by forwarding calls instead of cutting-and-pasting functionality:-

. . .
void DoSomething(string[], int, int)
void DoSomething(string[])
void DoSomething(IEnumerable<string>)
. . .

So, seriously, has that level of detail ever enabled you to solve a problem? Without knowing what the parameter values are how much do stack traces even tell you? Agreed, if all you’ve got is a crash dump then a stack trace is invaluable, but I’m talking about logging stack traces which by definition means that you’re already writing other diagnostic information too.

Design Smell?

I’ve always favoured keeping stack traces out of log files on the basis that they are of little value in comparison to other techniques, and so far I’ve found that I don’t miss them. In my experience, if the design of the code is right and the error message (e.g. exception message) is well written it should be fairly easy to reason about where in the code the problem is, which is effectively what a stack traces tells you. In short that means a simple GREP on the source code to find where the message is generated.

You might argue that a stack trace tells you that up front so why make more effort than necessary, which is of course true, but you’re also going to need the context, which a stack trace will not tell you unless it logs all its parameter values too. And for that we need to visit the log file, and if we’re going to do that how much time and effort are we really saving at the cost of extra background noise? More importantly this is the moment when the quality of our log message entries will begin to shine or we find ourselves lost in the trees looking for the wood. Hopefully during development you’ve already been dog-fooding your own logs to get a feel for how well you can solve real support problems when using them.

Test Infrastructure

The second part of the puzzle of avoiding needing all this gratuitous text is an ability to reproduce the problem easily within a debugger. Hopefully from the context you should be able to explore the problem in isolation - varying different inputs to see how the code is reacting. If the design is simple you should easily be able to step through an existing test case and see where the points of trouble might be, e.g. some missing or dodgy error handling.

At this stage, while the overarching goal is to fix the problem at hand, the fact that a bug has crept in means that the development process has failed to some degree and therefore I’d be taking this as an opportunity to compensate by doing a review. It’s likely I won’t action anything there-and-then, instead favouring to make some notes so that any eventual action can be triaged and prioritised separately.

Depending on the complexity of the system, this is the point at which I might rely on any custom tooling I’ve built to help isolate certain aspects of the system so that they can be exercised in a tightly controlled and deterministic environment, e.g. console app test harness that hosts the various services in-process.

Minimal Traces

What I despise most about many stack traces I see in forum posts is the sheer volume of noise. There is clearly some value in them, more so for “generic” unhandled exceptions like a NullReferenceException that have no message, but do they have to extend to reams and reams of text? When I log an exception I write the entire exception chain with both the type and the message; all on a single line. This is done using an extension method for the Exception type in C#. The same could easily be done for the stack trace, the only reason I haven’t done it is because I haven’t needed to, yet. But if I did write one what would it do?

Firstly I’d strip away all the arguments as they are fairly meaningless without their values. I’d also collapse all overloads into a single method as forwarding calls are uninteresting too. The bottom part of any stack trace is a load of boilerplate system code, so I’d chuck that away and make the entry point to my code the first interesting point of reference, which I should be able to find because assembly names and namespaces tend to follow a corporate convention. The same is probably true for the top of the stack, but the very first frame may be meaningful so perhaps I’d keep that, although if I had to keep just one method name it would be the last method of my code I’d keep as that is the first point that has any real meaning. Finally, I’d rotate what’s left and stitch it together with pipes, probably something like this (ignoring the unfortunate word-wrap):-

2001-01-01 ERR Unhandled exception - OrderController.GetOrder|>ProductService.FindProduct {NullReferenceException}

I definitely don’t need the full namespace names, just the class and method, although I’d argue that with decent method names even the classes might easily be inferred from just the method name and context. Doesn’t that look a whole lot less noisy?

Whilst I might not convince you to drop stack traces entirely from your logs, at least entertain the idea that you can represent them in a far more support-friendly fashion than what the runtime throws out by default.

Friday 22 November 2013

The 3 Faces of PowerShell Collections - 0, 1 & Many

There is a classic rule of thumb in programming that says there are only  three useful numbers - zero, one and many. I’ve found this concept very useful when writing tests as code that deals with collections or item counts sometimes need to handle these 3 cases in different ways. As a simple example imagine generating a log message about how many items you’re going to process. The lazy approach would be to just print the number and append a “(s)” to the noun to make it appear as though you’ve made an effort:-

Found 2 file(s) to process…

If you wanted to spell it out properly you’d write 3 separate messages:-

  1. No files need processing
  2. Found 1 file to process…
  3. Found 2 files to process…

A PowerShell Gotcha

This idea of 0, 1 & many is also the way I remember how PowerShell collections work when they are returned from a cmdlet. I was reminded of this idiom once again after debugging a colleague’s script that was failing because they had written this:-

$items = Get-SomeItems . . .

if ($items.Count -gt 0) {
. . .

For those not well versed in PowerShell this kind of construct will generate an error when no item or just 1 item is returned. The error will tell you “Count” is not a property on the variable - something like this in fact:-

Property 'Count' cannot be found on this object. Make sure that it exists.
At line:1 char:55
+ . . .
    + CategoryInfo : InvalidOperation: . . . 
    + FullyQualifiedErrorId : PropertyNotFoundStrict

You won’t see this error unless you have Strict Mode turned on (hence the PropertyNotFoundStrict in the error message). For one-liners this might be acceptable, but when I’m writing a production grade PowerShell script I always start it with these two lines (plus a few others that I covered in “PowerShell, Throwing Exceptions & Exit Codes”):-

Set-StrictMode -Version Latest
$ErrorActionPreference="stop"

For those used to the family of Visual Basic languages the former is akin to the “Option Explicit” statement you probably learned to add after misspelling variables names a few times and then scratched your head as you tried to work out what on earth was going on.

PowerShell Collections

To help illustrate these three manifestations of a collection you might come across we can create 3 folders - an empty one, one with a single file and one with many files [1]:-

PS C:\temp> mkdir Empty | Out-Null
PS C:\temp> mkdir Single | Out-Null
PS C:\temp> echo single > .\Single\one-file.txt
PS C:\temp> mkdir Many | Out-Null
PS C:\temp> echo many > .\Many\1st-file.txt
PS C:\temp> echo many > .\Many\2nd-file.txt

Now, using Get-ChildItem we can explore what happens by invoking the GetType() method in the resulting value from the cmdlet to see exactly what we’re getting [2]:-

PS> $items = Get-ChildItem Empty; $items.GetType()
You cannot call a method on a null-valued expression.

PS> $items = Get-ChildItem Single; $items.GetType()
IsPublic IsSerial Name     BaseType
-------- -------- ----     --------
True     True     FileInfo System.IO.FileSystemInfo

PS> $items = Get-ChildItem Many; $items.GetType()
IsPublic IsSerial Name     BaseType
-------- -------- ----     --------
True     True     Object[] System.Array

As you can see in the first case we get a null reference, or in PowerShell terms, a $null. In the second case we get a single item of the expected type, and in the third an array of objects. Only the final type, the array, will have a property called “Count” on it. Curiously enough, as you might have deduced from earlier, you don’t get a warning about a “null-valued expression” if you try and access the missing Count property on a $null value, you get the “invalid property” error instead:-

PS C:\temp> $null.Count
Property 'Count' cannot be found on this object. Make sure that it exists.

Forcing a ‘Many’ Result

The idiomatic way to deal with this in PowerShell is not to try and do it in the first place. It is expected that you will just create a pipeline and pass the objects along from one stage to the next letting the PowerShell machinery hide this idiosyncrasy for you:-

PS C:\temp> Get-ChildItem Empty | Measure-Object |
            Select Count
Count
-----
    0

However, if you do need to store the result in a variable and then act on it directly [3] you’ll want to ensure that the variable definitely contains a collection. And to do that you wrap the expression in “@(...)”, like so:-

PS> $items = @(Get-ChildItem Empty);
    Write-Output $items.Count
0
PS> $items = @(Get-ChildItem Single);
    Write-Output $items.Count
1
PS> $items = @(Get-ChildItem Many);
    Write-Output $items.Count
2

 

[1] Apologies for the old-skool syntax; I still work with a lot with batch files and the PowerShell syntax for creating directories just hasn’t bedded in yet. The blatant use of ECHO instead of Write-Output was me just being perversely consistent.

[2] Whilst Get-Member is the usual tool for inspecting the details of objects coming through a pipeline it will hide the different between a single value and a collection of values.

[3] For example diagnostic logging, which I tackled in “Logging & Redirection With PowerShell”.

Wednesday 13 November 2013

Self-Terminating From Inside a TCL Script DDE Handler

Like all good bugs, the one you discover is not always the one that is actually causing the real problem the user is reporting. In my last post “DDE XTYP_EXECUTE Command Corruption” I described a problem I ran into when sending a DDE XTYP_EXECUTE message from a Unicode server to an ANSI client. Whilst this had became a problem, it turned out this wasn’t the actual problem that the user had originally reported.

Hanging on Exit

The real problem the user was experiencing was that when they sent a DDE command to their TCL script to terminate itself, the calling script which was written in VBScript was timing out with a DMLERR_EXECACKTIMEOUT error. What made things more curious was that the user had managed to find a workaround using another DDE tool (a command line application) that did seem to terminate the TCL script without generating an error.

Although I knew nothing about TCL at all at that point, my spider-senses were already tingling when I saw this bit of the TCL script in their email:-

proc TclDdeServerHandler {args} {
  . . .
  switch -exact -- $cmd {
    . . .
    exit {
      exit
    }
  }
}

The code path for the “exit” command was causing the TCL script to terminate whilst still instead the DDE handler. Although it may not actually be a bad thing to do in a script I’ve always tended to try and let any stacks unwind before terminating a process or script to make sure the “runtime” remains in “a good place”. Maybe I’ve just spent too many hours porting native libraries that use “exit()” instead of “return” as an error handling strategy [1].

I raised this as a concern, but given the other developer knew TCL and I didn’t I was happy to accept their answer that this wasn’t an issue.

My First TCL Script

After taking a crash course in TCL, which really just involved hacking around the script I’d already been given, I managed to create a simple one that acted as a trivial DDE server to print a popular message:-

proc RunServer {} { 
  package require dde
  dde servername TestTopic
}

proc SayHello {} { 
  puts "Hello world"
}

RunServer
vwait forever

I ran this using TCLSH.EXE MyScript.tcl and poked it remotely using a similar nugget of VBScript:-

Dim client
Set client = CreateObject("DDECOMClient.DDEClient")

client.ExecuteTextCommand "TclEval","TestTopic",
                          "SayHello"

The hardest bit about getting this working was the making the script sit in a loop processing Windows messages instead of terminating, and that’s what the “
vwait forever” does. The only way to exit this though it to use Ctrl+C in the console.

To test the configurable timeout behaviour I’d added to my COM component I added a sleep in the SayHello function like so.

global alarm
proc sleep {time} { 
  after $time set alarm 1 
  vwait alarm
}
. . .
proc SayHello {} { 
  puts "Waiting..." 
  sleep 2000 
  puts "Hello world"
}

Reproducing the Real Issue

My “improved” DDE COM Component went back to the original developer so they could then bump the timeout themselves to something sensible in their script. They came straight back to to say that increasing the timeout didn’t work. They had bumped it up to 60 secs, which, after further discussion revealed was 59.99 secs longer than the operation should really take.

With a bit more TCL knowledge under my belt I started to hack around with their script, which took a while as I couldn’t even get it to run under TCLSH.EXE. A bit of random commenting out and I was finally able to reproduce the timeout problem. At this point I assumed the real issue might lie with some interaction between myself and the VBScript engine or be a subtle bug in my COM component as it was being hosted in-process and had already made the other DDE calls.

However what I couldn’t reproduce was their ability to terminate the script using another process. At first I used my own DDECmd tool as I like to dog-food my own stuff when possible. No go. Luckily they shipped me the execdde tool they were using and lo-and-behold it also failed with a timeout exception too. Huh?

Trust Nothing

At this point I was totally bemused and was sure I was a victim of some kind of environmental difference, after all, how else could the bug reporter experience one thing whilst I saw another unless there was a difference in the tools themselves or the way they were being used? Time to start again and re-evaluate the problem…

Luckily I had been sent a copy of the scripts being run by the other developer and so I started to look more closely at what they were doing. Whilst reviewing this I noticed that the call to this 3rd party execdde tool was being done in such a way as to ignore the return code from it. In fact the lines above had the error reporting in, but it was now commented out. In the heat of battle it’s all too easy to just keep trying lots of different things in the hope that something eventually works and then we quickly lose sight of what we have and haven’t tried up to that point.

This caused me to also re-evaluate my theory about calling “exit” from inside the TCL DDE handler and so I did some more Googling and came up with this variation of the TCL DDE handler that sets an exit “flag” instead, unwinds, and then finally exits by dropping out the bottom:-

set forever 1

proc TclDdeServerHandler {args} {
  . . .
  switch -exact -- $cmd {
  . . .
    exit    {
      global forever
      set forever 0 
    } 
  }
}

package require dde 1.3
dde servername -handler ::TclDdeServerHandler TestTopic
vwait forever

This worked a treat. Although there was much gnashing of teeth wondering why the “forever” variable wasn’t changing. Apparently I need to tell TCL that the “forever” variable I was changing was the global one, instead of changing a local one.

TCLSH vs WISH

That was the end of it, or so I thought. All along there had actually been an “environmental” difference between what I was working with and what the other developer was using - the TCL interpreter. I didn’t realise that TCLSH.EXE and WISH.EXE are two slightly different variations of the TCL scripting host. I had wondered why I needed to comment out the line “console show” when trying to run their script, but just as they had done I moved on and forgotten to evaluate how I had got there.

The main difference, at least as far as what I proposed, was that when my script is run under WISH.EXE it doesn’t actually terminate, oops! Although I didn’t manage to confirm it, my suspicion was that WISH behaves more GUI like and so it probably has an impliedvwait __forever__” at the end (but waiting on some internal variable instead). The solution of course is as simple as appending the manual call to “exit” that we took out of the DDE handler to the bottom of the script:-

. . .
package require dde 1.3
dde servername -handler ::TclDdeServerHandler TestTopic

vwait forever
exit

Is there a Bug?

I managed to find a workaround that allows the person that reported the problem to do what it was they needed to do. Sadly I don’t know whether their original idea was sound (being able to call exit directly) or if it’s a bug in the TCL interpreter or DDE package. Presumably being open source I have the opportunity to download the sources, build and debug it myself. Maybe one day I’ll have the time.

 

[1] Back in the mid 1990’s when the JPEG library was in its infancy I had the pleasure of taking this Unix-style C library that just called exit() all over the place when anything failed and tried to turn it into a DLL to run under 16-bit Windows. Because it was going to be part of a bigger application it had to return an error code instead of just bailing out; along with dealing with NEAR and FAR pointers, etc.

Tuesday 12 November 2013

DDE XTYP_EXECUTE Command Corruption

A few months back I had a request to add support for the XTYP_EXECUTE transaction type to my COM based DDE client. The question came from someone who was using the component via VBScript to automate a TCL script which in turn was automating some other processes! My underlying C++ based DDE library already had support for this, along with my DDECmd tool so I thought it was going to be a simple job. What I didn’t expect was to get into the murky depths of what happens when you mix-and-match ANSI [1] and Unicode build DDE clients and servers...

Adding XTYP_EXECUTE Support

Due to the nature of COM the API to my COMDDEClient component is essentially Unicode. When I went into my code to check how the command strings were handled in my C++ library I discovered (after spelunking the last 10 years of my VSS repo) that when I ported the library to allow it to be dual build (ANSI & Unicode) I just took the command string as is (either char or wchar_t) and pushed it out directly as the value for the XTYP_EXECUTE transaction data.

The XTYP_EXECUTE DDE transaction type is an oddity in that the “format” argument to the DdeClientTransaction() function is ignored [2]. The format of this command is documented as being a string, but the exact semantics around whether it’s ANSI or Unicode appear left unsaid. I therefore decided to correct what I thought was my naive implementation and allow the format to be specified by the caller along with the value - this felt more like a belt-and-braces approach.

Naturally when I came to implement my new ExecuteTextCommand() method on the COM DDE server IDDEConversation interface, I followed the same pattern I had used in the rest of the COM server and defaulted to CF_TEXT as the wire format [3]. I tested it using the new support I’d added to my DDECmd tool via the --format switch and thought everything was great.

Impedance Mismatch

Quite unexpectedly I quickly got a reply from the original poster to say that it wasn’t working. After doing the obvious checks that the build I provided worked as expected (still using my own DDECmd tool as the test harness), I took a crash course in TCL. I put together a simple TCL script that acted as a DDE server and printed the command string send to it. When I tried it out I noticed I didn’t get what I expected, the string appeared to be empty, huh?

Naturally I Googled “TCL XTYP_EXECUTE” looking for someone who’s had the same issue in the past. Nothing. But I did find something better - the source code to TCL. It took seconds to track down “win/tclWinDde.c” which contains the implementation of the TCL DDE support and it shows that TCL does some shenanigans to try and guess whether the command string is ANSI or Unicode text (quite a recent change). The implementation makes sense given the fact that the uFmt parameter is documented as being ignored. What then occurred to me as I was reading the TCL source code (which is written in C) was that it was ANSI specific. I was actually looking at slightly older code it later transpired, but that was enough for the light bulb in head to go on.

A consequence of me using my own tools to test my XTYP_EXECUTE support was that I had actually only tested matched build pairs of the DDE client and server, i.e. ANSI <-> ANSI and Unicode <-> Unicode. What I had not tested was mixing-and-matching the two. I quickly discovered that one permutation always worked (ANSI client to Unicode server) but not the other way (a Unicode client sending CF_TEXT to an ANSI server failed).

Guess what, my DDECOMClient component was a Unicode build DDE client sending CF_TEXT command strings to a TCL DDE server that was an ANSI build. Here is a little table showing the results of my mix-and-match tests:-

Client Sending Server Receives Works?
ANSI CF_TEXT Unicode CF_UNICODETEXT Yes
ANSI CF_UNICODETEXT Unicode CF_UNICODETEXT Yes
Unicode CF_TEXT ANSI ??? No
Unicode CF_UNICODETEXT ANSI CF_TEXT Yes

Forward Compatible Only

I looked at the command string buffer received by the DDE server in the debugger and as far as I can tell DDE will attempt to convert the command string based on the target build of the DDE server, which it knows based on whether you called the ANSI or Unicode version of DdeInitialize() [4]. That means it will automatically convert between CF_TEXT and CF_UNICODETEXT format command strings to ensure interoperability of direct ports of DDE components from ANSI to Unicode.

In fact it does even better than that because it will also correctly convert a CF_TEXT format command sent from a Unicode build DDE client to a Unicode build DDE server. The scenario where it does fail though, and the one that I created by accident through trying to be flexible, is sending a CF_TEXT format command string from a Unicode build DDE client to an ANSI build DDE server.

Once I discovered that DDE was already doing The Right Thing I just backed out all my DDE library changes and went back to the implementation I had before! And hey presto, it now works.

 

[1] For “ANSI” you should probably read “Multi-byte Character Set” (MBCS), but that’s awkward to type and so I’m going to stick with the much simpler (and much abused) term ANSI. This means I’m now as guilty as many others about using this term incorrectly. Sorry Raymond.

[2] Not only is this documented but I also observed it myself whilst investigating my later changes on Windows XP - the uFormat argument always seemed to reach the DDE Server callback proc as 0 no matter what I set it to in the client.

[3] Given the uses for DDE that I know of (mostly financial and simple automation) CF_TEXT is more than adequate and is the lowest common denominator. Nobody has written to me yet complaining that it needs to support requests in CF_UNICODETEXT format.

[4] It is not obvious from looking at the DdeInitialize() signature that the function even has an ANSI and Unicode variant because there are no direct string parameters.

Friday 8 November 2013

Extract Method is About More Than Code Reuse

I read an interesting article recently from Reginald Braithwaite titled “Defactoring”. Sadly the article was published in a format where no comments can be added directly, so I’m going to publish mine here instead.

The premise of the article was about performing the reverse action of “factoring” on a codebase - essentially manually inlining methods again. The reason the author cited for doing this was largely because the promise of reuse, for which the original factoring had apparently taken place, had never materialised. My problem with this assertion is that the code was factored out solely for the purpose of reuse [1], which is rarely the reason I do it. For me, after having factored code out, if I should happen to need to reuse it later then it’s merely a fortuitous side-effect that it has already been simplified.

Introduce Explaining Variable

In Martin Fowler’s book on Refactoring, one of the items (Ch. 6, p124) is about breaking up long expressions into smaller ones and giving the result a name, i.e. using a variable (or constant [2]), e.g. instead of writing:-

var pt = new Point(((right-left)/2)+left 
                   ((bottom-top)/2)+top);

...you could break the expression down and name different parts:-

var width = right - left;
var height = bottom - top;
var centreX = left + (width / 2);
var centreY = top + (height / 2);
var centrePoint = new Point(centreX, centreY);

OK, so I’ve gone to the extreme here to make a point (pun intended) and in the real world the expressions are usually longer and have a few nested uses of the ternary operator thrown in for good measure.

The main reason I would do this decomposition is to explain my thinking. If there is a bug in the original example it might not be obvious what that bug is because you can’t see how I derived the expression. In the latter case, by trying to name the elements of the expression I’m cementing in my own mind exactly how it is I’ve arrived at the solution.

Show Your Workings

With my eldest child just about to go through his exams, I’m reminded about how important it is to “show your workings”. At school it’s all very well knowing you know something, but in an exam it’s important to show the examiner that you know it too. Sometimes we have to learn it the hard way by staring at a maths problem we know we’ve done wrong but can’t see where we went astray because all we wrote down was the final answer.

Magic Numbers

If you take an expression and reduce it down to its smallest elements you eventually arrive at creating names for literal values, or as they are often referred to: magic numbers. The most primitive of these are boolean values and anyone who has come across code that uses the CreateEvent function in the Windows API will have met this kind of code:-

CreateEvent(..., true, false, ...);

You have to look up the documentation to see what the arguments refer to; some simple local constants can make this much more readable at the call site:-

const bool manualEvent = true;
const bool notSignalled = false;

CreateEvent(..., manualEvent, notSignalled, ...);

Introduce Explaining Method

The alternative to introducing explaining variables would be to introduce explaining methods where we wrap up the functionality into lots of small, well named methods instead. Going back to our earlier example we could refactor our expression this way instead:-

public int CentreX(left, right)
{
  return ((right - left) / 2) + left;
}

Then we can use the methods directly:-

var centrePoint = new Point(CentreX(left, right),
                            CentreY(top, bottom));

On such as simple and obvious example as this it probably seems overkill, but I’ve often come across many other small expressions that just seem to take way too much brain power to understand - both what they represent and, more importantly, if they’re correct.

Turning once again to the Refactoring book and the Extract Method item in particular (Ch. 6, p 110) we find the following advice in the Motivation section:-

“If extracting improves clarity, do it, even if the name is longer than the code you have extracted”.

Too Many Layers of Abstraction?

Once again I find a criticism of this degree of breakdown being that it adds too many levels to the code. I suspect the people who say this seem hell bent on trying to squeeze as much code as possible into the smallest area on the screen. When there are too many layers the calls are across multiple classes, whereas what I’m talking about means lots of little private methods in the same source file.

Additionally, the point of naming is to convey something to the reader, and that’s often hard to do in programming, so much so I can understand why it seems easier to just not do it.

Decades ago there might also have been an argument against doing this on the grounds that it affects performance, and it may well do in dynamic languages, but modern statically compiled languages can easily inline this stuff.

There is a famous quote by Sir Tony Hoare that goes:-

“One way is to make it so simple that there are obviously no deficiencies, and the other way is to make it so complicated that there are no obvious deficiencies”

To me, breaking a problem down as much as possible helps to create a program that approaches the former state rather than the latter.

 

[1] Of course it’s entirely plausible that the author has chosen to focus solely on this particular use case for factoring out methods and is perfectly aware of others. Either way, I’m going to add my own £0.02 to the debate.

[2] See the Single Assignment Pattern.