CodeSOD: Vestigial Organs

Matt inherited some C# code that reads from a JSON config file.

public ServerJsonLoader(string configFile)

{

	using (StreamReader reader = File.OpenText(configFile))

	{

		JObject config = ... //snip



		if (config.GetValue("inputs") != null)

		{

			this.mixedConfig = config;



			//Inputs

			var inputs = ... //snip



			//Outputs

			if (config.GetValue("outputs") != null)

			{

				var outputs = ... //snip

			}                

		}

		else

		{                 

			if(config.GetValue("inputs") != null)

				this.serverConfig = config;



			if (config.GetValue("outputs") != null)

				this.clientConfig = config; 

			

		}                    

	}

}

We open the file as a stream, and then hand it off to a JObject, which handles the parsing for us. Then, if it has a value "inputs", we store the config as mixedConfig, and handle the inputs. Then, if it also has "outputs", we also process the outputs.

If we don't have an "inputs", we go down the else path, where we check: if we have "inputs", we store the config in serverConfig, and if we have "outputs", we store the config in clientConfig.

It pains me to say that this convoluted logic isn't useless. It may not even be wrong. In the case where there are no "inputs", but there are "outputs", we'll store the config in clientConfig.

What we have here is a terrible way to write a simple concept: if we have only outputs, we must be a client. I suspect that they once wanted it to also be true that if you had only inputs, you were a server. And if you had both, you were "mixed". Then, something changed about their rules or their config, and the server-only configuration became a little vestigial branch off this chain of ifs, never to be executed again.

[Advertisement]
ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.