The Daily WTF
Follow
CodeSOD: Do a Lot to Do Nothing
A legacy finance application contains a C# method, ValueAGPFund, which is criticized for its numerous side effects. This method modifies its input parameters and internal class members, performing multiple distinct operations. It also interacts with a method called CheckPreviousValuationIfRequired. This latter method is designed to retrieve and process previous valuation data based on specific parameters.A significant issue arises from a circular dependency where ValueAGPFund calls CheckPreviousValuationIfRequired, which in turn calls ValueAGPFund. Furthermore, the return statement in CheckPreviousValuationIfRequired is flawed, leading to a NullReferenceException when it attempts to access a null value. This error likely masks the intended logic, which was presumably to recursively call ValueAGPFund only when prior valuation data existed. The current implementation means this function either returns null or throws an exception.Despite these apparent flaws, the application reportedly functions correctly. The submitter suspects that the problematic code, CheckPreviousValuationIfRequired and its interaction with ValueAGPFund, might be redundant. However, they are hesitant to remove it without confirming if it has any unforeseen side effects or if other parts of the system rely on the thrown NullReferenceException. The submitter sarcastically notes that unit tests would typically mitigate such risks, implying their absence or inadequacy. The core problem lies in the complex and potentially broken logic of these interdependent methods.