FSharpx.Core.Datastructures default format?

[Jack Fox]

Three-part question:

1) Some structures have type defined within namespace FSharpx.DataStructures and module values defined under module with same name as type. Example:

namespace FSharpx.DataStructures

type DList<'a> =

...

[<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]

module DList =

    let empty<'a> : DList<'a> = Nil

...

Others are defined with the type and module values all within the named module. Example: BootstrappedQueue

I would prefer standardizing on the first method. What is the opinion of core team members?

2) Not all module values that could be defined inline are. I assume inline module values are preferred when possible. Thoughts?

3) What is the core team memberss opinions about breaking backwards compatability to achieve uniformness in the data structures API (especially in regards to item 1)?

[Mauricio Scheffer]

1) I agree with you, but in the particular case of BootstrappedQueue, ImplicitQueue and AltBinaryRandomAccessList there's a comment justifying the use of static members, and the expected let-bound functions are also provided anyway, so callers wouldn't see any difference, right?

Are there any other cases of this, affecting callers?

2) I don't think making everything inline is beneficial. The usual rule of thumb is to make small functions inline, and personally I'd tend towards not making things inline unless it's proven that it makes a positive difference.

3) I tend to favor breaking backwards compatibility, but whenever possible first mark things as [<Obsolete>] and document it, and leave the obsolete things around for a while before finally removing them.

Cheers

Mauricio

[Jack Fox]

1) I'm pretty sure I've solved this problem. Mark the required static members internal, then create exposed member modules calling the hidden static modules passing "this". Yes, let-bound module functions too, so it acts just like MS.Core.Collections structures like "list", having both module and type values.

I'm attaching a new version of AltBinaryRandomAccessList (which I have not submitted yet) which does this and implements IRandomAccessList<'a> (also not submitted yet) so you can see what I'm talking about. The attached changes do not break any backward compatibility. I don't know if anyone will ever find the interface useful, but it does enforce uniformity within structure families.

2) Yes, I should have noted only the very small inline functions. I think the impact is very negligible.


There is so much good stuff in FSharpx , have you guys ever considered recruiting someone to act as product manager and document everything? Sure would be nice to be able to use VS help viewer.

[Steffen Forkmann]

Hi,

I added these changes as a pull request. Please continue the discussion there.

https://github.com/fsharp/fsharpx/pull/89

Cheers,

Steffen

2012/9/18 Jack Fox <jack...@gmail.com>

[Jack Fox]

Steffen,

Do not put these files in the pull request. I only posted these files for discussion purposes.

The interface for Deques is changed in Interfaces.fs and the Deques will not build without the corresponding changes in their files.

I'll have a pull request ready in a few days with new deques and random access lists.

Jack

[Steffen Forkmann]

Hi,

I didn't merge the pull request to the master. It's just a pull request.

The idea behind github's pull request feature is to make such discussions easier since everybody can see the diff and comment on the changes and much more.

[Steffen Forkmann]

... and I noticed the interface changes. I added only the RandomAccessList to the pull request.

2012/9/18 Steffen Forkmann <sfor...@gmail.com>

[Jack Fox]

I did some testing and I am able to change all the deque structures to the preferred format without breaking backward compatibility, except for one item, which is if someone is using  the "new" constructor. However, the new constructor should not be used at all. If I knew of a way to hide it I would. Using the constructor requires knowledge of the internal structure, which most users would not have or bother to learn. It's much easier and safer to use one of the numerous other ways of instantiating a deque. You can still use the constructor under the DataStructures namespace instead of under the module.

[Steffen Forkmann]

I am ok with breaking the constructor. Please send me the files or a pull request.

Cheers,

Steffen

[Jack Fox]

Working on 2 new random access lists right now. I will send everything
when they are done.