CodeSOD: An Obsolete Approach

Marcus's team was restructuring the API, and the architect thus wanted a number of methods marked obsolete, to encourage developers to move to the new version of the API. So the architect created a Jira task, assigned it to a dev, and moved on.

Somehow, this C# code got committed and merged, despite being code reviewed:

public static void OBSOLETE_PopulateOfferNodes(Ecometry.New.Model.CategoryHierarchyNode _mainNode, ref Ecometry.New.Model.CategoryHierarchyNode _defaultOffersNode, ref Ecometry.New.Model.CategoryHierarchyNode _foundOffersNode){foreach (Ecometry.New.Model.CategoryHierarchyNode offersNode in _mainNode.Children){if (offersNode.Category.Name.ToUpper().Trim().Equals("_DEFAULT")) _defaultOffersNode = offersNode;if (offersNode.Category.Name.ToUpper().Trim().Equals(Ecometry.New.Impl.ExecutionContextFactory.ExecutionContext.CurrentCart.SourceCode.Code.ToUpper().Trim()) && Snapshot.OfferRelatedUtilities.isDateOK2Show(offersNode)) _foundOffersNode = offersNode;if (_defaultOffersNode != null && _foundOffersNode != null) break;}return;}

Now, what the architect had meant was that the developer should use the [Obsolete] attribute, which lets you annotate methods like so:

[Obsolete("Foo is deprecated, please use Bar instead")]public void Foo() {}

What the developer heard, instead, was that they should do a find and replace of every use of the method name and prepend OBSOLETE_ to it.

The annotation has a number of advantages: unlike the name, it produces a compiler warning, and that warning can be promoted to a compiler error in future versions, without fully removing the method. It also helpfully avoids this drunken form of Hungarian notation that sneaks what is essentially version information into the method name.

This is the first step on a path that ends with methods named New_UseThisOne_Foo().

All that aside, the if statements are a mouthful in there. They're not a WTF, but... boy, they could probably be simplified for readability. I suspect that's part of why this method is obsolete.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!