CodeSOD: The Truth and the Truth

When Andy inherited some C# code from a contracting firm, he gave it a quick skim. He saw a bunch of methods with names like IsAvailable or CanPerform..., but he also saw that it was essentially random as to whether or not these methods returned bool or string.

That didn't seem like a good thing, so he started to take a deeper look, and that's when he found this.

public ActionResult EditGroup(Group group){string fleetSuccess = string.Empty;bool success = false;if(action != null){fleetSuccess = updateGroup(group);}else{fleetSuccess = Boolean.TrueString;}success = updateExternalGroup(group);fleetSuccess += "&&&" + success;if (fleetSuccess.ToLower().Equals("true&&&true")){GetActivityDataFromService(group, false);}return Json(fleetSuccess, JsonRequestBehavior.AllowGet);}

So, updateGroup returns a string containing a boolean (at least, we hope it contains a boolean). updateExternalGroup returns an actual boolean. If both of these things are true, than we want to invoke GetActivityDataFromService.

Clearly, the only way to do this comparison is to force everything into being a string, with a &&& jammed in the middle as a spacer. Uh, for readability, I guess? Maybe? I almost suspect someone thought they were inventing their own "and" operator and didn't want it to conflict with & or &&.

Or maybe, maybe their code was read aloud by Jeff Goldblum. "True, and-and-and true!" It's very clear they didn't think about whether or not they should do this.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!