CodeSOD: Listicle
Pardon the clickbait headline. We're only going to look at one method to tell if an item is in a list. A bad one.
Andrew M. inherited some software that tracks metrics. There are three general categories of metrics- "MS", "FN", and "CM". Each of these categories contains a number of specific metrics, each assigned its own ID.
So, the previous developer needed to write a function that answered this simple question: given a metric's ID, is it an "MS", "FN", or "CM" metric?
public static string GetModel(int metricId){ string caMSMetricIdList = "29477|29478|29479|29480|29481|29482|29483|29484|29485|29486|29487|29488|29489|29490|29491|29492|29493|30157|30732|30735|30738"; string caFNMetricIdList = "29411|29412|29413|29414|29415|29416|29417|29418|29419|29420|29421|29422|29423|29424|29425|29426|29427|29428|29429|29430|29431|29432|29433|29434|29435|29436|29437|29438|29439|29440|29459|29460|29461|29462|29463|29464|29465|29466|29467|30181|30183|30185|30187|30189|30436|30607|30608|30609|30610|30611|30612|30615|30616|30731|30734|30737"; string caCMMetricIdList = "29005|29364|29365|29366|29367|29368|29369|29370|29371|29372|29373|29374|29375|29376|29377|29378|29379|29380|29381|29382|29383|29384|29385|29386|29387|29388|29389|29390|29391|30435|30553|30554|30555|30556|30557|30558|30730|30733|30736"; if (caMSMetricIdList.Contains(metricId.ToString())) { return "MS"; } if (caFNMetricIdList.Contains(metricId.ToString())) { return "FN"; } if (caCMMetricIdList.Contains(metricId.ToString())) { return "CM"; } return "";}On the bright side, these aren't technically "magic numbers", and since the fields are delimited by pipes, there won't be any lurking "run-over" bugs, where the substring of two IDs mashed together is a valid ID.
On the dark side, Andrew wasn't the first person to inherit this. A long line of developers preceded them, and each of them simply appended each new set of metric IDs to the ever growing lists. Andrew replaced this with a simple lookup using a dictionary.
[Advertisement] Manage IT infrastructure as code across all environments with Puppet. Puppet Enterprise now offers more control and insight, with role-based access control, activity logging and all-new Puppet Apps. Start your free trial today!