fslang-suggestions icon indicating copy to clipboard operation
fslang-suggestions copied to clipboard

Make it more practical for an API to have type, module and/or union case with the same name

Open dsyme opened this issue 3 years ago • 16 comments

Make it fully practical to use an API that has a type, module and union cases with the same name without opening that module.

Consider this:

module Domain 
type TodoID = TodoID of int
module TodoID = let value (TodoID i) = i

In this situation it is currently not possible to reference the module from a different source file using a fully qualified name. For example, try to reference it from a different file using a fully qualified name:

module DTO_broken
let ti = Domain.TodoID 1
let i1 = Domain.TodoID.value ti // error FS0039: The field, constructor or member 'value' is not defined.
let i2 = Domain.TodoIDModule.value ti // error FS0039: The value, constructor, namespace or type 'TodoIDModule' is not defined

Code does not compile.

Relevant dotnet/fsharp issues

  • https://github.com/dotnet/fsharp/issues/9807#issuecomment-679239494
  • https://github.com/dotnet/fsharp/issues/8424
  • https://github.com/dotnet/fsharp/issues/13122

dsyme avatar Aug 31 '20 17:08 dsyme

See also https://github.com/dotnet/fsharp/issues/4185

dsyme avatar Sep 01 '20 12:09 dsyme

There are also cases where having a union type and union case with the same name causes issues, see https://github.com/dotnet/fsharp/issues/8875

dsyme avatar Sep 01 '20 12:09 dsyme

Better to add to the style guide "avoid giving a DU case the same name as its type".

Conflating the name of a case and its type is a conceptual confusion which shouldn't be allowed.

(As an analog, in C# you cannot call a nested inner class the same name as the outer class.)

charlesroddie avatar Sep 03 '20 07:09 charlesroddie

Conflating the name of a case and its type is a conceptual confusion which shouldn't be allowed.

Except it's extremely common and convenient in single-case DU scenarios.

kerams avatar Sep 03 '20 09:09 kerams

Common yes, so that's why I would suggest a style guide recommendation rather than a warning, together with not adding extra support (as in this issue). There are a lot of common pitfalls that the style guide is good at helping reduce.

A "single-case DU", i.e. a DU with a single case where there is no intention to support additional cases in future, was a poor choice (compared to, for example, a record with a single field) that was used once on a popular website and ended up spreading.

There are several things wrong with that pattern, but the important thing here is that reusing the name is done to allow developers to mentally conflate the case and the type. That is a problem because there is a high level essential difference between those things in F#.

Note that this consideration doesn't apply to

type TodoID = ...
module TodoID = ...

Here there is a difference between the type and the module, but the difference is not a high-level essential one as both things can contain values (static members and let bindings respectively) that are accessed in the same way. The difference is not important to consuming code, so it's very reasonable to use the same name here.

charlesroddie avatar Sep 03 '20 09:09 charlesroddie

that was used once on a popular website and ended up spreading.

Scott is certainly part of it, but I think the main reason it has become somewhat of a norm is because it's based on a norm elsewhere, namely Haskell's newtype.

Tarmil avatar Sep 03 '20 11:09 Tarmil

There are several things wrong with that pattern

:point_up: That's a big understatement. :slightly_smiling_face:

pblasucci avatar Sep 03 '20 11:09 pblasucci

I found the simplest way to avoid confusion like this is to use the RequireQualifiedAccess attribute:

module Domain 
[<RequireQualifiedAccess>]
type TodoID = TodoID of int
module TodoID = begin
  let value (TodoID.TodoID i) = i
end

zpodlovics avatar Sep 05 '20 08:09 zpodlovics

Given the number of reports this is clearly a pattern people are trying to do and would just expect to work, so this should just get done.

cartermp avatar Sep 05 '20 17:09 cartermp

There are several things wrong with that pattern

☝️ That's a big understatement. 🙂

I often use single case DUs, what are the problems with that pattern ?

davidglassborow avatar Sep 08 '20 16:09 davidglassborow

I often use single case DUs, what are the problems with that pattern ?

I can't speak for Paul, but I can tell you why I do not use the pattern.

  • Having to unwrap everywhere
  • Less visibility into actual data, especially when many of these are composed into a type
  • Taken to logical extreme, case data becomes private, making it like an (immutable) object instead of a value
    • By that, I mean only owning module can operate on it, so access patterns become more OO-like.

Basically more ceremonial overhead than just using a string to represent an email, for example. Then rather than encoding conformance of value into a type (e.g. ValidatedEmail), my approach is to take that same validation code I would put into an SCU "constructor" and make it a separate function that is composable with others. I place the overall validation function in the execution path so that it fails if values do not conform. Subsequent functions can assume validity. That way the value can always stay in a form that is easy to work.

Much as I like to believe SCUs are square pegging, it is possible I just don't have the use cases where the required extra lengths it takes are justified, whereas others may. And for good or ill, people do use them.

kspeakman avatar Sep 08 '20 20:09 kspeakman

I often use single case DUs, what are the problems with that pattern ?

  1. Name conflation as described above
  2. Only one case so no discrimination so doesn't match the name "Discriminated Union"
  3. Additional scaffolding for matching cases created and used unnecessarily

Records do the same thing without the above problems (type [<RequireQualifiedAccess>] TodoID = { Value:int }). Replacing a record type P = { X:int; Y:int } with a single case DU type P = P of x:int * y:int is always possible and always worse.

charlesroddie avatar Sep 08 '20 21:09 charlesroddie

It's a pretty common pattern. I normally privatize the constructor as well and have the module create the type.

module Domain 
type TodoID = private TodoID of int
module TodoID = 
	let create = if validate i then Ok i else Error "bla"

let consumeTodo (TodoId id) = 
  // blah blah

Swoorup avatar Sep 09 '20 06:09 Swoorup

I often use single case DUs, what are the problems with that pattern ?

So, there are really two aspects to this: technical and philosophical.

From a purely technical perspective, for the same amount of source code:

  • a record provides more features
  • a single-case union generates more boilerplate

Admittedly, the second point is of debatable importance (especially if the union/record is only ever consumed from F# code). However, given the sort of helper functions one tends to see accompanying a single-case union, one wonders why not just choose the record-based approach.

But, let's leave the technical aspects aside for a moment. The philosophical aspects are much more interesting. Generally, one sees single-case unions cropping up in two different scenarios:

  1. something not unlike Haskell's newtype functionality
  2. a rough approximation of what's known as a "value object" in Domain-Driven Design

N.B. the main distinction between these two is the latter seeks to incorporate "correct by construction" semantics into the type itself; meanwhile, the former is simply trying to put a domain-centric mask on the underlying -- usually primitive -- type

As far as emulating newtype:

  • if the underlying type is a numeric primitive, just use units of measure (more info: here)
  • for other underlying types, it's less overhead to use a generalised "tagging" approach (c.f.UMX)
  • there's also the afore-mentioned technical differences

With respect to "value objects", typically, the main reason to use a union or a record is to get the structural semantics for free. Also, you can have the same "potentially zero/low allocation" struct-ness with either approach. So, in that regard, is doesn't really matter which one you choose. Regardless of union or record, one still hs to:

  1. Hide the constructor
  2. Provide explicit construction functions
  3. Decide if/when/how to expose the underlying value (hint: what if you didn't expose it?)

And, as @kspeakman pointed out, there's not always a benefit to this approach. Sometimes it's just easier and better to model the result of correct-by-construction and "trust" the appropriate safeguards are in place (aside: I imagine library versus application is a big driver here).

Anyway, I think there's merit to each approach (which is why I'm in favor of the original language suggestion). But in my experience, the problem is more "developers copy-pasting something without understanding the trade-offs" than it is the approach itself.

pblasucci avatar Sep 09 '20 07:09 pblasucci

Thanks for the thoughts on this all. I have started using UMX as well which works well.

Looking at the code generated, apart from an extra _DebugDisplay member, and the tag constant, the underlying representation is virtually the same between SCU and Record:

https://sharplab.io/#v2:DYLgZgzgNAJiDUAfALgTwA4FMAEBVAdgJYD2+2AvHkadsWNgGoCGwArpiIfsgLABQ/NFmwAlTAGNiAJxgVsAb0Yt22ENi7JsAX35A===

davidglassborow avatar Sep 09 '20 08:09 davidglassborow

Just want to add that this would also be helpful for any DU with a case that shares a name in the same namespace (sharplab):

module Mod =
    type InnerDU =
        | Foo
        | Bar
        
    type OuterDU =        
        | InnerDU of InnerDU
        | Baz
    
let fn =
    function
    | Mod.InnerDU.Foo -> "foo" // error FS1127: This is not a constructor or literal, or a constructor is being used incorrectly
    | Mod.InnerDU.Bar -> "bar"

open Mod

let fn2 =
    function
    | InnerDU.Foo -> "foo" // no error
    | InnerDU.Bar -> "bar"

gsuuon avatar Feb 16 '21 22:02 gsuuon

Better to add to the style guide "avoid giving a DU case the same name as its type".

Conflating the name of a case and its type is a conceptual confusion which shouldn't be allowed.

@charlesroddie I'll keep in mind the single-field record pattern, but I would still like to avoid confusion when naming a module the same as a DU type within it.

LyndonGingerich avatar Nov 29 '22 15:11 LyndonGingerich