3
\$\begingroup\$

Note that this code uses C#7 functionality like local methods and ValueTuple

I am programming an HMI for an industrial machine. Part of the HMI's task is to read the machine settings and present them in better human understandable concepts. In this case a Material and its corresponding MaterialWebfeedSettings.

In the following method I extract information from a List of MachineSetting which contains the information needed to create a Material instance and MaterialWebfeedSettings instances for each WebfeedModule on the machine.

The Dictionary<ModuleTypes, List<MachineProperty>> WebfeedModuleSettingProperties contains a List of MachineProperty for each possible ModuleTypes that is aso a WebfeedModule.

List<(ModuleTypes type, Int32 sequenceNumber)> WebfeedModules contains all the WebfeedModules on the machine.

My main concern with this method is readability and coding style, and performance to a lesser extend.

private static List<(ModuleTypes type, Int32 sequenceNumber)> WebfeedModules;

private static Dictionary<ModuleTypes, List<MachineProperty>> WebfeedModuleSettingProperties;

private static (Material, List<MaterialWebfeedSettings>) ExtractMaterial(List<MachineSetting> machineSettings)
    {
        Func<MachineSetting, Boolean> IsWebThickness = s => s.MachinePropertyID == MachineProperties.MainCtrl_rWebThicknessSetting;
        Func<MachineSetting, Boolean> IsWebStretch = s => s.MachinePropertyID == MachineProperties.MainCtrl_rWebStretchSetting;
        Func<MachineSetting, Boolean> IsTaperStartTens = s => s.MachinePropertyID == MachineProperties.Rewind_rTaperStartTens;
        Func<MachineSetting, Boolean> IsMinStopDiameter = s => s.MachinePropertyID == MachineProperties.Unwind_rMinStopDiameter;

        Single thickness = Single.Parse(machineSettings.First(IsWebThickness).ValueEnd);
        Single stretch = Single.Parse(machineSettings.First(IsWebStretch).ValueEnd);
        Single windingTension = Single.Parse(machineSettings.First(IsTaperStartTens).ValueEnd);
        Single minStopDiameter = Single.Parse(machineSettings.First(IsMinStopDiameter).ValueEnd);

        Material material = new Material("", thickness, stretch, true, windingTension, minStopDiameter);

        List<MaterialWebfeedSettings> settings = new List<MaterialWebfeedSettings>();

        foreach ((ModuleTypes type, Int32 sequenceNumber) module in WebfeedModules)
        {
            Func<MachineSetting, Boolean> IsWebtension = s => s.MachinePropertyID == WebfeedModuleSettingProperties[module.type][Webtension].ID;
            Func<MachineSetting, Boolean> IsWebtensionControl = s => s.MachinePropertyID == WebfeedModuleSettingProperties[module.type][WebtensionControl].ID;
            Func<MachineSetting, Boolean> IsSpeedOffset = s => s.MachinePropertyID == WebfeedModuleSettingProperties[module.type][SpeedOffset].ID;

            String settingWebtensionValue = machineSettings.FirstOrDefault(IsWebtension)?.ValueEnd;
            String settingWebTensionControlValue = machineSettings.FirstOrDefault(IsWebtensionControl)?.ValueEnd;
            String settingSpeedOffsetValue = machineSettings.FirstOrDefault(IsSpeedOffset)?.ValueEnd;

            String defaultWebtensionValue = WebfeedModuleSettingProperties[module.type][Webtension].DefaultRecipeValue;
            String defaultWebtensionControlValue = WebfeedModuleSettingProperties[module.type][WebtensionControl].DefaultRecipeValue;
            String defaultSpeedOffsetValue = WebfeedModuleSettingProperties[module.type][SpeedOffset].DefaultRecipeValue;

            Int32 webTension = Int32.TryParse(settingWebtensionValue, out Int32 webTensionValue) ? webTensionValue : Int32.Parse(defaultWebtensionValue);
            Boolean closedLoop = Boolean.TryParse(settingWebTensionControlValue, out Boolean webTensionControlValue) ? webTensionControlValue : Boolean.Parse(defaultWebtensionControlValue);
            Single speedOffset = Single.TryParse(settingSpeedOffsetValue, out Single speedOffsetValue) ? speedOffsetValue : Single.Parse(defaultSpeedOffsetValue);

            settings.Add(new MaterialWebfeedSettings(material, (WebFeedModule)(Int32)module.type, module.sequenceNumber, webTension, closedLoop, speedOffset));
        }

        return (material, settings);
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ Well, you're using value tuple but there are no local functions, lambdas like IsWebThickness were already possible in C# 6. \$\endgroup\$ – t3chb0t Apr 6 '17 at 15:09
1
\$\begingroup\$
Func<MachineSetting, Boolean> IsWebThickness = s => s.MachinePropertyID == MachineProperties.MainCtrl_rWebThicknessSetting;

This is not a local function, it's a variable of Func type and as such it should be camelCase and not PascalCase. This is a local fuction:

bool IsWebThickness(MachineSetting s) => s.MachinePropertyID == MachineProperties.MainCtrl_rWebThicknessSetting;

It's as if you placed a normal method inside another method but without the access modifier.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.