Initail build of duplex steam

[peternewell]

Initial build of duplex steam - addition of control commands only

https://blueprints.launchpad.net/or/+spec/duplex-steam-locomotives

It is desired to merge this so that control keys are "reserved".

Additional functionality supporting this will be added later under a separate PR

[cesarBLG]

Just two style fixes, sorry for not having spotted them before

[cesarBLG]

Suggested change

	if (Receiver == null) return;
	{
	Receiver.SteamBoosterChangeTo(ToState, Target);
	}
	Receiver?.SteamBoosterChangeTo(ToState, Target);

[peternewell]

I am not as comfortable with this change.

Firstly I think that long hand code is sometimes more "readable" then short hand code.

Secondly there are a number of other similar entries in the other controllers. Hence for consistency these should be changed as well.

So I would prefer to leave this as it is, and somebody else can refactor all the code to be similar at a later date.

[cesarBLG]

I'm sorry but I won't approve it as is. It looks confusing to me. In particular, at first glance your code seems to do

if (Receiver == null)
{
    Receiver.SteamBoosterChangeTo(ToState, Target);
}

until you notice the "return" statement. The curly braces are completely misleading.

The ?. operator is my preferred choice in this case, and is also recommended by ORMT (see http://www.elvastower.com/forums/index.php?/topic/35509-cs-null-conditional-operator/). It is also used elsewhere in Commands.cs, so your argument about consistency is not really valid. In my opinion, new code should use it whenever possible.

However, my intention is not to force you to use that syntax. It was just my suggestion. I have no problem if you prefer to use "standard" syntax, as long as the misleading braces are removed. Other acceptable solutions for me are:

if (Receiver == null) return;
Receiver.SteamBoosterChangeTo(ToState, Target);

or

if (Receiver != null)
{
    Receiver.SteamBoosterChangeTo(ToState, Target);
}

or

if (Receiver != null) Receiver.SteamBoosterChangeTo(ToState, Target);

[peternewell]

Ok, I have adopted one of your suggested solutions.

[cesarBLG]

I'm sorry for being so nitpicking, but the indentation of Receiver.SteamBoosterChangeTo(ToState, Target); is still incorrect. I think it is important that we follow the coding guidelines to make code more readable for others. I could have corrected it myself, but you disabled "allow edits from maintainers".

[peternewell]

I have moved it across now. Thanks for checking.

[cesarBLG]

Thanks, and sorry for the inconvenience of the multiple style suggestions.

[peternewell]

Thanks Cesar.