Avoiding another boolean check

A developer wrote some code like this:

	public void RunScheduledJob()
	{
		if (_loggingEnabled)
			Log.Write("Main Job Complete", _category, _severity, _customData);
	}

	public void LogWrite(string message)
	{
		if (_loggingEnabled)
			Log.Write(message, _category, _severity, _customData);
	}

I’ve removed some extra code to make the example clearer, but the RunScheduledJob would do something then write to a log if the feature is enabled. The LogWriteMethod writes to a log if the feature is enabled.

Although it’s not a major improvement, the obvious thing to do would be to use the LogWrite method in the RunScheduledJob method like this:

	public void RunScheduledJob()
	{
		LogWrite("Main Job Complete");
	}

	public void LogWrite(string message)
	{
		if (_loggingEnabled)
			Log.Write(message, _category, _severity, _customData);
	}

So the reviewing developer Jim, pointed this out to Rich:

Jim: Could you call the LogWrite method here?
Rich: I could do but it would then evaluate _loggingEnabled twice for no reason.

Now, Jim and I were baffled what he meant. Even if it did have to check _loggingEnabled, it is a simple boolean so would only waste 1 millisecond to evaluate again. There’s no question of performance here; only clarity.

Rich then suggested this code as an improvement:

	public void RunScheduledJob()
	{
		if (_loggingEnabled)
			LogWriteNoCheck("Main Job Complete");
	}

	public void LogWrite(string message)
	{
		if (_loggingEnabled)
			LogWriteNoCheck(message);
	}

	private void LogWriteNoCheck(string message)
	{
		Log.Write(message, _category, _severity, _customData);
	}

So we have lost a bit of clarity.

It’s weird how sometimes developers have moments of madness and over-complicate simple things. This particular developer has 30 years programming experience!