CodeSOD: Popping a Plister
We live in a brave new world. Microsoft, over the past few years has emphasized, more and more, a cross-platform, open-source approach. So, for example, if you were developing something in .NET today, it's not unreasonable that you might want to parse a PList file- the OSX/NextStep/GNUStep configuration file format.
But let's rewind, oh, say, five years. An Anonymous reader found a third-party library in their .NET application. It never passed through any review or acquisition process- it was simply dropped in by another developer. Despite being a .NET library, it uses PLists as its configuration format- despite .NET offering a perfectly good in-built format. Of course, this C# code isn't what we'd call good code, and thus one is left with an impression that someone hastily ported an Objective-C library without really thinking about what they were doing.
For example, perhaps you have an object that you want to convert to a binary PList file. Do you, perhaps, use overriding and polymorphism to create methods which can handle this? Do you perhaps use generics? Or do you ignore all of the benefits of a type system and use a case statement and compare against the type of an object as a string?
private static byte[] composeBinary(object obj){ byte[] value; switch (obj.GetType().ToString()) { case "System.Collections.Generic.Dictionary`2[System.String,System.Object]": value = writeBinaryDictionary((Dictionary<string, object>)obj); return value; case "System.Collections.Generic.List`1[System.Object]": value = composeBinaryArray((List<object>)obj); return value; case "System.Byte[]": value = writeBinaryByteArray((byte[])obj); return value; case "System.Double": value = writeBinaryDouble((double)obj); return value; case "System.Int32": value = writeBinaryInteger((int)obj, true); return value; case "System.String": value = writeBinaryString((string)obj, true); return value; case "System.DateTime": value = writeBinaryDate((DateTime)obj); return value; case "System.Boolean": value = writeBinaryBool((bool)obj); return value; default: return new byte[0]; }}
Honestly, the thing that bothers me most here is that they're both setting the value variable and returning from each branch. Do one or the other, but not both.
[Advertisement] High availability, Load-balanced or Basic - design your own Universal Package Manager, allow the enterprise to scale as you grow. Download and see for yourself!