When it comes to software, the concept of time can cause problems. There’s actually some really interesting scenarios, but even in simple applications, some developers really struggle with simple concepts.

In terms of standard problems, you can have problems where the client and server times can be out. This can just be because they are set incorrectly, or maybe are using a different timezone. As a developer, if you are looking at times in log files across the client and server, it can cause confusion if the timestamps are out. A common thing I have seen is that some servers don’t use Daylight Savings Time we have in the UK, but the client times often do. So the server can be an hour out.

Daylight savings time is interesting as time shifts forward or backwards one hour. So time isn’t linear.

I recall reading a blog about time by Jon Skeet who then discussed how if you are using historical dates, the time can also suddenly change. Like if a country switches to a different calendar system entirely, so moving a day could suddenly jump in years to align with the new system.
Computerphile have a discussion on this The Problem with Time & Timezones – Computerphile

Leap Years

We once had a leap year bug because someone created a new date using the current day and month, and added a year. So when it was 29th Feb, it tried to create a date of 29th Feb for next year which wasn’t a valid date. So the feature crashed. Everyone was panicking trying to rush out a fix, but then we realised we could only get the fix out to our customers tomorrow, and the bug wouldn’t happen. Not for another 4 years anyway. It was hilarious

-1

One weird mistake I saw recently, is that a developer defined a variable and set it to 5. The code they wrote was supposed to make sure that we never make an API call more than once every 5 minutes. However, they then minused 1, so were checking every 4 minutes instead.

var minimumNumberOfSecondsRequiredBetweenEachAPICall = (NumberOfMinutesRequiredBetweenEachAPICall - 1) * 60;

Ages

You would think everyone would understand the concept of ages since everyone has an age and it increases by 1 every time you have a birthday. However, many developers seem to struggle with the concept. The last implementation I saw had the following:

int age = DateTime.Now.Year - dateOfBirth.Year;

So it can be one year out because it basically assumes your birthday is 1st January.

It reminds me of an exchange on Twitter that I saw years ago. It was in the context of football.

PA: Why are Arsenal paying £25m for a 29 year old striker?
G: he’s 28 btw
PA: He’s a lot nearer to 29 than 28, that’s a fact
G: He’s 28, that’s a fact
PA: Why am I not surprised that fractions are beyond you. The day after his birthday, he is no longer 28.
G: He’s 28 until he becomes 29. That’s how it works
PA: Perhaps if you had paid more attention in Maths lessons? You might remember “round up or down to the nearest whole number”
G: He’s 28. That’s a fact.
PA: No, it is not. £1.75 is not one pound. You don’t even understand what a fact is now.
G: Until he is 29, he is 28.

When it is the next day after your birth, are you 1 day old? technically you could just be a minute old but claim you are 1 day old.

My instinct to perform mathematics on dates would be to use an existing date library. Another developer tried to make something themselves. This seemed a bit complex to me, but I think it actually worked, or at least seemed reasonable for how they wanted to use it.

public static double AgeInYearsAtDate(DateTime effectiveDate, DateTime dateOfBirth)
{
        double daysInYear = 365.25;
        int completeYears = Age.GetYears(dateOfBirth, effectiveDate);

        dateOfBirth = dateOfBirth.AddYears(completeYears);

        double proportion = effectiveDate == dateOfBirth ? 0 : Age.GetDays(dateOfBirth, effectiveDate) / daysInYear;

        return completeYears + proportion;
        }

        public static string ConvertCurrentAgeToYearsAndMonths(double age)
        {
                int monthsInYear = 12;
                int years = (int)age;
                int months = (int)Math.Round((age - (int)age) * monthsInYear);

        return $"{years} year{(years == 1 ? String.Empty : "s")} and {months} month{(months == 1 ? String.Empty : "s")}";
        }

Ages Part 2

Another developer was testing his age code and wrote this:

new object[]
            {
                new DateTime(2010, 05, 31),
                new DateTime(2009, 06, 01),
                AgeRange.UpToOneYear,
                "52 weeks and 0 days"
            },

If there’s 52 weeks in a year, then is that 52 weeks? kinda looks 1 day short to me. Time is mental isn’t it?

Here is a selection of poor code design that I have come across over the years.

Opacity Slider

Sometimes, programmers write simple programs to help them, or their team. When I first joined this company, they were using a more basic version of a source control like SVN, and someone had designed a program to help with Code Reviews. On the tool, they had a slider that changes the opacity of the dialog, so you can make it see-through. It seemed like a case of the developer simply doing it because they could, and not because anyone actually found the feature useful. I suppose you could be more creative if it’s only used internally, and not for a customer; but still incredibly pointless.

Law of Demeter

Law of Demeter, also known as the principle of least knowledge, advocates for a design where objects are loosely coupled and only communicate with their immediate acquaintances. This means that a method in a class should only call methods on:

• Its direct components.
• The object itself.
• Objects passed as parameters.
• Any objects it creates.

In practice, this means you shouldn’t have a massive chain of methods/properties.

One example of breaking this law that I saw in our code looked like this:

responseMessage.acknowledgements.conveyingTransmission.controlActEvent.reason[0].detectedEvent.code.code

Awful.

Partial Record

One of our modules can potentially show a large amount of data. The data can come from the local record, or if there is a sharing agreement, it can come from other companies that they share with. There are certain screens where you don’t need all the information, so to try and cut down loading times, we have this concept of a Partial record and a Full Record.

	public IRecordPartial Partial
	{
		get
		{
			return IsFullLoaded ? Local : EnsurePartial();
		}
	}

The logic, and wording gets confusing real fast. The property above is called Partial, but there is a check for IsFullLoaded which implies that Local can be Full, and not just Partial like the property says. When you look further into the code, Local might not even be local because it can contain shared. Mindbending.

PercentNull

Coming up with descriptive names is always a challenge in programming, and there is occasions where naming can be ambiguous. However, I have no idea what a method called PercentNull does here:

transactionOut.Surname = helperFacade.PercentNull(customerDetail.Surname);

If it assigning the result to Surname, then it should be returning text. Nothing immediately obvious comes to mind if you are passing in Surname to a method called PercentNull and getting a Surname from that. So it’s not like it is returning a percent number if it can, or Null if it cannot. Or returning the percentage that the text contains whitespace. 🤷

How High

	public enum SeverityScale
	{
		None = 0,
		Low = 1,
		Medium = 2,
		High = 3,
		ExtraHigh = 5,
		VeryHigh = 6
	}

We have this enum to represent the Severity. It makes sense until you get further than High. Extra High makes sense to be more severe than High, but should Very High be higher than Extra High? Extra and Very sound like synonyms. You need something more extreme like Extremely High to make it immediately obvious what the ranking is.

Focussed or not?

	public void SetSearchBox()
	{
		SearchFocused = false;
		SearchFocused = true;
	}

When you see code like the above, it is most likely that the code is doing something weird, and because no one worked out how to fix the original issue, you then end up writing more weird code to work around that. These “hacks” just stack up and are hard to remove. If you try to do the right/honourable thing and remove a hack, then you will probably see some strange behaviour which then means you remove more code, and maybe more code when you find more problems. Repeat until you are left with the original issue which you then have to fix.

So how can setting a property to false, then immediately setting it to true actually do something? Probably if the property has loads of logic and side-effects. Probably best not to look. 🙈

Random Parameter

		public Organisation(bool randomParamToChangeSignatureForFasterConstruction)
		{
			// Had to create a new constructor that doesn't initialise internal state, param isn't used.
		}

The thing that is scary about this strange code, is that it was written by one of our smartest developers. I have no idea why he had to create an extra constructor. Presumably the parameter is there because the “default” constructor already existed. Intentionally not initialising data sounds like a recipe for bugs though. Maybe it needs a redesign.

Slow Request

I remember my Dad telling me a story of a software team putting a delay into their code. Then each month, they would simply reduce the delay a bit and tell their managers they have been working hard on performance improvements.

if (Helper.SlowRequest)
	Thread.Sleep(15000);

I found the above code in our codebase but it relies on a configuration value being present in the config file. It was present and set to true by default for Developers though so would always be slow. Changing the value to false would speed it up, but you have to know that it exists to change it.

<add key="SlowRequest" value="true"/>

Although it doesn’t affect our customers, there’s always the chance something will go wrong one day and it could affect them.

Boolean “Mode”

If you want to handle many options, you often use an “enum”. Using a “Boolean” which represents 2 values (true/false) is a very weird choice…

 <param name="mode">If set to <c>true</c> then [mode] is a delete operation, else add, edit and none operations</param>

so

true = delete

false = add OR edit OR none.

If you put the wrong value, then there’s an if statement after. So if you say it’s a delete and it isn’t, then things don’t get deleted.

		if(!mode)
		{
			if (@event.updateMode != vocUpdateMode.delete)
			{
			}
			else
			{
				if (@event.updateMode == vocUpdateMode.delete)

Not Supported Property

	public virtual String UploadedBy
	{
		get { return _document.Observation.EnteredByUser.DisplayName; }
		set { throw new NotSupportedException("Setting UploadedBy is not supported"); }
	}

Isn’t that Set a complete dick-move. If you call it, it will crash. If the setter wasn’t there at all, you would know it shouldn’t be set.

I guess it could be designed with the expectation that you would override the property. However, I thought it wouldn’t be set correctly, because the “get” returns a massive chain of properties. So it’s not just the case of setting a variable, it’s actually document.Observation.EnteredByUser.DisplayName that needs to be set, and that is breaking the Law of Demeter anyway.

Mutual Exclusive

This is gonna end in tears

private Boolean _isSingleClick;
private Boolean _isDoubleClicked;

Not only are there better ways than detecting clicks, when you have multiple variables tracking similar concepts like this, you can easily end up in invalid states. If _isDoubleClicked is true, then you would expect _isSingleClick to always be false. But it is easy to make a mistake in the code and not set it which then leads to a bug.

Notifications

		public bool IsNotificationConfigEmailEnabled()
		{
			if (!_configSmsEnabled.HasValue)
				_configSmsEnabled = NotificationConfiguration.IsEmailEnabled;

			return _configSmsEnabled.Value;
		}

The fact that this code had been there years means it should work. But when the property is supposed to be checking if Email is enabled but the code only looks at SMS enabled; then who knows how it works.

Resizing

int parentSize = Parent != null
                ? Parent.Width
                : Screen.PrimaryScreen.Bounds.Width;
 
var availableSpace = parentSize - _clientLocation.Y - _yOffset;

What a nonsense calculation that is! We want to calculate the height of a pop up box and make sure it can fit within the window. So we look at the width of control, or maybe the width of the monitor that they might not be using (if they actually have the program on their secondary monitor), then minus the Y coordinate of the window which would be related to height, and not width.

Splitting

Sometimes programmers like jamming as much code as they can on what is technically a single line. It is an absolute nightmare to debug chained logic like this:

return
	RequiresSingleItemOrder(item, customerRequestsSplit)
	?
	null
	:
	batchOrders
	.FirstOrDefault(
		orderInstance =>
			(orderInstance.IssueMethod != IssueMethod.Electronic || orderInstance.Items.Count() < 4)
			&&
			!orderInstance.Items.Any(m => RequiresSingleItemOrder(m, customerRequestsSplit))
			&&
			orderInstance.IssueMethod == issueMethod
			&&
			orderInstance.OrderType == OrderType
			&&
			orderInstance.IsUrgent == isUrgent
			&&
			(!ShouldSeparateControlledItems
			||
			orderInstance.AnyControlledItems == item.IsControlledItem)
			&&
			orderInstance.Warehouse == Warehouse
			&&
			!Authorisation.Separator.ShouldBeSeparated(authoriser: authoriser, orderAuthoriser: orderInstance.Authoriser)
			&&
			(!ShouldSeparatePrivateItems
			||
			orderInstance.IsPrivate == isPrivate)
			&&
			MatchesForRepeatOrdering(item, orderInstance)
			&&
			NullAndNonsequentialEqual(separationTypeIds, orderInstance.SeparationTypeIds));

Funny Youtube Comment

I saw this comment on a programming video on YouTube. It is remarking on how you can write really confusing code as a way to increase your job security because you can be in charge of code that only you can read:

And remember, kids – if you nest multiple null coalescing operators into a single line of poly-nested ternary operators, that’s called “job security” – cause ain’t no one wanna maintain that code when you’re gone.

return a> b ? a < c ? a != d ? e ?? f ?? 0 : f ?? g ?? : 0 e ?? g ?? 0;

Typescript Example

Years ago, we started rewriting our C# program using Web Technologies. Since everyone was new to Javascript and Typescript, everyone wrote awful code. But then you didn’t know if it was good or bad. One of the first bits of code I saw when I joined their team was this:

export const Actions: { [key: string]: <T extends {}> (value: T) => IAction<T> } = {
 setModuleName: <T extends {}>(value: T): IAction<T> => CreateAction<T>(ActionTypes.setModuleName, value),
};

Don’t ask me what it says, because I have no idea.

Contradiction

InsertUpdateResourceImmutableProperties

It’s so frustrating to read, it makes you want to punch someone. Immutability means it shouldn’t change after it has been created. So how can you Update something Immutable? Luckily it has a comment explaining it:

   -- If we are about to insert the most upto date version of the resource, then we should update the
    -- resources immutable properties, because they might have changed in the source system. (Even though they shouldn't).

Well, I am still none-the-wiser.

Boolean Extensions

If you have a boolean variable and you want to check it is false, you can just write:

			bool example = false;

			if (example == false)
			{
			}

Now with this handy extension method:

public static class BooleanExtensions
{
	public static bool IsFalse(this bool boolValue)
	{
		return !boolValue;
	}
}

You can now write:

bool example = false;

if (example.IsFalse())
{
}

What’s the point in that? No advantage really is there?

Not Set Up

		get
		{
			if (_setup)
			{
				_setup = false;
				return true;
			}

			return _noDocumentsInError;
		}

In the get property, we check the value of _setup. If true then we set it to false, and the overall result returns true. However, if we immediately call the same property, it will just return the value of _noDocumentsInError instead. It’s bad design to have side-effects in a get. Gets are supposed to return a value, and not set things. Then we seem to have different fields tracking different concepts which just looks like it will be prone to errors.

Reload

	public void ReloadTriggers()
		{
			// To be implemented
			return;
		}

This code doesn’t even need a return statement. It is just there for the bantz. When this code is in a module that is very error prone, and you have a method that is not implemented and does nothing at all, then doesn’t give a good impression does it?

Dialog Events

            _msgBox.ShowDialog();
        	return _continueWithSelection ? DialogResult.OK : DialogResult.Cancel;
}
 
private void Cancel_Selection(object sender, EventArgs e)
{
       _continueWithSelection = false;
       _msgBox.Dispose();
}

You can get the dialog result when the dialog is closing. However, this developer has decided to create their own event handlers for the button clicks, then using the variable to decide if it is a DialogResult.OK or Cancel.

NoEncryptionEncryptor

new NoEncryptionEncryptor() 

Is this an Encryptor or not?🤔

SillyException

You throw an Exception when something bad has happened. However, in our code there is a concept of SillyException where you are supposed to ignore it because apparently, it isn’t a real error. It’s only used in a specific part of our codebase though so goes undiscovered for ages. A colleague Michael found it again and we were reminiscing on a bug I found.

Michael 11:33: 
if (mapped is SillyException) return 0;         // Avoid throwing the SillyException 

Me 11:33: 
I love sillyexception 

Michael 11:33: 
/// <summary>Represents a data provider exception that doesn't indicate an actual error, 
/// and should be ignored.</summary> 
[Serializable] 
public class SillyException 

Me 11:34: 
I remember David going on about SillyExceptions and it took me ages to realise he was referring to an actual thing 

Michael 11:35: 
"I keep getting this SillyException" 
what kind of exception is it David? 
IT'S A SILLYEXCEPTION 
YES, BUT WAT KIND 

Me 11:35: 
yeah, it did go something like that 

Michael 11:35: 
haha 

Me 11:37: 
I think it was around the time he spent weeks investigating that bug I found that no one else could recreate 
and I thought he had worked it out when he said he was getting a SillyException 

 lame async call

var result = client.BeginUploadFile((Stream)payload, endpoint);
bool uploadCompleted = false;
while (!uploadCompleted)
{
	uploadCompleted = true;
	if (!result.IsCompleted)
	{
		uploadCompleted = false;
	}
}
if (!uploadCompleted)
{
	throw new Exception($"Failure occurred while uploading the data");
}
client.EndUploadFile(result);

weird that they set it to true, then set it back to false. Then it will never get to the exception, it will just be an infinite loop because the check is outside the loop that will only exit if it is complete.

Is this just a reimplementation of a lame async call?