2
\$\begingroup\$

I have a sample code and some question.

Simple type property

member val CabinetNetworkState : uint8 = 0uy with get, set
member this.CabinetNetworkStateAsValues : (CabinetNetworkStateHosts * uint8) array =
        let cabinetNetworkStateMap = new ResizeArray<(CabinetNetworkStateHosts * uint8)> ()

        let extNetworkState = (this.CabinetNetworkState >>> 0) &&& uint8 2
        match Enum.IsDefined(typeof<CabinetNetworkState>, extNetworkState) with
        | true ->  cabinetNetworkStateMap.Add (CabinetNetworkStateHosts.EXT, extNetworkState)
        | false -> cabinetNetworkStateMap.Add (CabinetNetworkStateHosts.EXT, 0uy)

        let canNetworkState = (this.CabinetNetworkState >>> 3) &&& uint8 2
        match Enum.IsDefined(typeof<CabinetNetworkState>, canNetworkState) with
        | true ->  cabinetNetworkStateMap.Add (CabinetNetworkStateHosts.CAN, canNetworkState)
        | false -> cabinetNetworkStateMap.Add (CabinetNetworkStateHosts.CAN, 0uy)

        let net1NetworkState = (this.CabinetNetworkState >>> 5) &&& uint8 2
        match Enum.IsDefined(typeof<CabinetNetworkState>, net1NetworkState) with
        | true ->  cabinetNetworkStateMap.Add (CabinetNetworkStateHosts.NET1, net1NetworkState)
        | false -> cabinetNetworkStateMap.Add (CabinetNetworkStateHosts.NET1, 0uy)

        let net2NetworkState = (this.CabinetNetworkState >>> 7) &&& uint8 2
        match Enum.IsDefined(typeof<CabinetNetworkState>, net2NetworkState) with
        | true ->  cabinetNetworkStateMap.Add (CabinetNetworkStateHosts.CAN, net2NetworkState)
        | false -> cabinetNetworkStateMap.Add (CabinetNetworkStateHosts.CAN, 0uy)

        cabinetNetworkStateMap.ToArray()

What it does it converts a value into array of bit value and enum that value represents My question is should I use Map instead of tuple array, or it does not matter?

\$\endgroup\$
2
\$\begingroup\$

A Map would have a roughly constant time lookup on keys but if there only going to be 4 entries then checking each item in an array will probably be quicker anyway.

I think a much bigger issue is the repetition in the code and use of a ResizeArray to write in an imperative style when that can be easily avoided. Both of those issues are covered in this refactor (note that the names I used are quite arbitrary and you can probably come up with better ones):

member this.CabinetNetworkStateAsValues : (CabinetNetworkStateHosts * uint8) array =
    let makeStatePair state stateType =
        let networkState = (this.CabinetNetworkState >>> state) &&& 2uy
        if Enum.IsDefined(typeof<CabinetNetworkState>, networkState)
        then stateType, networkState
        else stateType, 0uy
    [|
        makeStatePair 0 CabinetNetworkStateHosts.EXT 
        makeStatePair 3 CabinetNetworkStateHosts.CAN
        makeStatePair 5 CabinetNetworkStateHosts.NET1
        makeStatePair 7 CabinetNetworkStateHosts.CAN
    |]
\$\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.