1
\$\begingroup\$

I have a situation where I'm stitching three different unions into a triplet tuple for building a map/dictionary of strongly typed values keyed on a string.

One of the union cases is very large, the second has three options, and the third has only two. Right now I loop through the large with reflection, and have six yield statements to handle the combinations of the second and third unions.

My code works fine as far as I can tell so far, but I don't quite like having yield in my code six times. How can I better represent this situation?

*If needed I can post the code for the unions too.

**Note that (RawOreName ore).Value is just a string.

let OreDataMap = 
    [
        for oreType in FSharpType.GetUnionCases typeof<EveOnline.OreDomain.Types.OreType> do
            let ore = FSharpValue.MakeUnion (oreType, [| |]) |> unbox

            yield (RawOreName ore).Value, (ore, Common, IsCompressed)
            yield (RawOreName ore).Value, (ore, Uncommon, IsCompressed)
            yield (RawOreName ore).Value, (ore, Rare, IsCompressed)
            yield (RawOreName ore).Value, (ore, Common, IsNotCompressed)
            yield (RawOreName ore).Value, (ore, Uncommon, IsNotCompressed)
            yield (RawOreName ore).Value, (ore, Rare, IsNotCompressed)
    ]
    |> Map.ofList
\$\endgroup\$
1
\$\begingroup\$

Why not use nested loops?

let OreDataMap = 
    [
        for common in [ Common; Uncommon; Rare ] do
            for compressed in [ IsCompressed; IsNotCompressed] do
                for oreType in FSharpType.GetUnionCases typeof<EveOnline.OreDomain.Types.OreType> do
                    let ore = FSharpValue.MakeUnion (oreType, [| |]) |> unbox
                    yield (RawOreName ore).Value, (ore, common, compressed)
    ]
    |> Map.ofList

That's probably the idiomatic way, but if you don't like excessive indentation you can always align your successive for loops - I think this looks nicer. Also, you can replace the last do with -> to eliminate the need for the yield keyword.

let OreDataMap = 
    [
        for common in [ Common; Uncommon; Rare ] do
        for compressed in [ IsCompressed; IsNotCompressed] do
        for oreType in FSharpType.GetUnionCases typeof<EveOnline.OreDomain.Types.OreType> ->
            let ore = FSharpValue.MakeUnion (oreType, [| |]) |> unbox
            (RawOreName ore).Value, (ore, common, compressed)
    ]
    |> Map.ofList
\$\endgroup\$
  • \$\begingroup\$ Oh! Actually I meant to explain why I specifically didn't use loops. The second and third unions are not ever going to change, so the extra loops are really superfluous. =) \$\endgroup\$ – Ken Mar 21 '15 at 2:43
  • 1
    \$\begingroup\$ I realize they're not going to change, but it avoids writing the yield 6 times, like you said. \$\endgroup\$ – luksan Mar 21 '15 at 2:47
  • \$\begingroup\$ You're totally correct in that. Thanks for the suggestion! \$\endgroup\$ – Ken Mar 21 '15 at 2:47
  • \$\begingroup\$ You could use ‘yield!‘ to avoid six yields and duplication of calling RawOreName. More lines though. \$\endgroup\$ – Robert Jeppesen Mar 21 '15 at 7:24
1
\$\begingroup\$

You could do a couple of nested for .. do expressions and just yield out once?

\$\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.