FEEDBACK: Dictionary mapping redesign

[James Gregory]

Hello everyone,

I've been working on something that I've been promising to do for a long time, rewrite our <map>/IDictionary functionality. It's not a shiny new feature or anything like that, but it's a nasty area of our user interface and codebase as a whole that has needed redoing for a long time.

I've put a branch up on the github repository with the work so far. This is very much a work in progress, and several things are unimplemented (most attributes, for example) but the general structure is present. I'd appreciate it if anyone who has some spare time would take a look, especially people who've had problems with <map>/IDictionary's in the past.

Branch: http://github.com/jagregory/fluent-nhibernate/tree/map-refactor

The refactoring is based on the following principal: infer or assume as much as possible, while providing a way for the user to customise as much as needed.

The HasMany and HasManyToMany dictionary overloads now return a new builder instance, instead of the general collection builder as before. This new builder is tailored specifically to dictionary mappings, and thus is much more specialised (and simpler in implementation). This is also why several features are currently unavailable, because they need to be reimplemented in the new builder (I'm deliberately keeping the classes separate, to avoid the mess we have currently).

Indirectly due to these changes, the following methods have been deprecated and/or removed entirely:

• AsMap
• AsEntityMap
• AsTernaryAssociation
• AsSimpleAssocation
• ParentKeyColumn (now just Key())

The mappings that all those methods created are entirely doable using the new interface. When these changes get merged into master, removed methods may be reintroduced and marked as obsolete rather than being removed entirely.

A few examples:

HasMany(x => x.Comments);

Where Comments is an IDictionary<string, string> would create a <map> with an <element>, with Key and Value as the column names.

HasMany(x => x.Comments)

  .Element("Content");

Same as above, except with the element column overridden.

HasMany(x => x.Comments)

  .Index("Author");

  

Same as above, except the index (dictionary key) column is overridden.

There's similar overloads for CompositeIndex, CompositeElement, OneToMany, and ManyToMany. Each method has a couple of simplistic signatures (string for column name, generic type for column type) and a lambda-based signature for advanced configuration.

HasMany(x => x.Comments)

  .Index(ix =>

  {

    ix.AsManyToMany();

    ix.Column("one");

    ix.Column("two");

    ix.ForeignKey("FK_something");

  });

You get the idea.

Feedback is much appreciated, and none of this is set in stone (or complete) yet.

[Nick Webb]

Solid work!

--
You received this message because you are subscribed to the Google Groups "Fluent NHibernate" group.
To post to this group, send email to fluent-n...@googlegroups.com.
To unsubscribe from this group, send email to fluent-nhibern...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/fluent-nhibernate?hl=en.

[James Gregory]

Thanks :)

[James Gregory]

UPDATE:

I've just pushed a load of commits that bring the new MapBuilder stuff in sync with the old map functionality in terms of available attributes and what not. Everything you could do before should now be doable in the new style; namely, the Inverse, Access, Cascade properties are now available, along with the rest.

That should be it really, feature wise I think this is done. Just looking for feedback as to whether this'll work for people or not.

[Jason Dentler]

I've been immersed in XML mappings for the better part of a year. I realize I'm warped and may not be the intended audience for this project anymore. 

In any case, I like it, but I'm not sure I like my reason. The new syntax is a little easier to translate to / from XML mappings, so a beginner will have an easier time mimicking some XML mapping without first understanding the more abstract mapping concept and vocabulary.

- Jason

[James Gregory]

Maps are complicated, and creating specialised concepts on top just made them even more complicated. I've never really been interested in having FNH be XML-in-code, but there are occasions when keeping it relatively close is beneficial.

For me, what I'd like to see is people having to call as few of the chained methods as possible. Have sensible defaults for as much as possible, and require very little (or none if possible) configuration to get up and running; if people want to customise types, column names, whatever, that's their prerogative but by default it should just work.

So what the redesign gives you is:

• a simpler interface for people that don't know, or don't care, how maps work at the NH level - string values are mapped as elements by default, entity key's as index-many-to-many, etc...
• full control over the mapping for advanced users that do know how maps are put together, and want to customise things to their heart's content

Hopefully those goals have been achieved. :)

[Rasmoo]

It looks really nice and accessible. One minor nitpick though:

When writing the hbm.xml files, it's fairly obvious when index is a
collection index, and when it's a database index. However, I might now
write:

Map(x => x.Name)
.Index("IX_SomeTable_Name");

HasMany(x => x.Comments)
.Index("Author");

Which causes me (and maybe just me) some cognitive dissonance. I know
it does not match the NHibernate vocabulary, but perhaps something
like CollectionIndex or IndexColumn would create the right
association?

On Aug 2, 11:07 pm, James Gregory <jagregory....@gmail.com> wrote:
> Hello everyone,
>
> I've been working on something that I've been promising to do for a long
> time, rewrite our <map>/IDictionary functionality. It's not a shiny new
> feature or anything like that, but it's a nasty area of our user interface
> and codebase as a whole that has needed redoing for a long time.
>
> I've put a branch up on the github repository with the work so far. This is
> very much a work in progress, and several things are unimplemented (most
> attributes, for example) but the general structure is present. I'd
> appreciate it if anyone who has some spare time would take a look,
> especially people who've had problems with <map>/IDictionary's in the past.
>
> Branch:http://github.com/jagregory/fluent-nhibernate/tree/map-refactor
>
> The refactoring is based on the following principal: infer or assume as much
> as possible, while providing a way for the user to customise as much as
> needed.
>
> The HasMany and HasManyToMany dictionary overloads now return a new builder
> instance, instead of the general collection builder as before. This new
> builder is tailored specifically to dictionary mappings, and thus is much
> more specialised (and simpler in implementation). This is also why several
> features are currently unavailable, because they need to be reimplemented in
> the new builder (I'm deliberately keeping the classes separate, to avoid the
> mess we have currently).
>
> Indirectly due to these changes, the following methods have been deprecated
> and/or removed entirely:
>

>    - AsMap
>    - AsEntityMap
>    - AsTernaryAssociation
>    - AsSimpleAssocation
>    - ParentKeyColumn (now just Key())

>
> The mappings that all those methods created are entirely doable using the
> new interface. When these changes get merged into master, removed methods
> may be reintroduced and marked as obsolete rather than being removed
> entirely.
>

> *A few examples:*

[James Gregory]

It's a hard one, and something that thoroughly annoys me about nhibernate maps. They're consistent with the other collections in naming, but the element behaviour/intent is different.

<map> : Dictionary
<key> : Foreign key
<index> : Dictionary.Key
<element> : Dictionary.Value

I'd like to rename Index to Key and Element to Value, but I think that might be too different (and Key could then be confused for the <key> element).

I'm open to suggestions though.

[Nick Webb]

Fluent NHibernate is, after all, a big facade of NHibernate's xml mappings, no?  Being a facade, I would not concern myself with how literal the fluent methods resemble the hbm context, as you said it yourself: you aren't necessarily looking to recreate the xml in c#.  Instead, I'd follow a philosophy of how the methods best represent how the object property is being mapped.  That's my opinion, anyway. 

So you're example of Index -> Key and Element -> Value seems very appropriate, especially for those not interested in learning how to rebuild an hbm map the 'fluent' way.  

To continue this approach: 

Index = Key

Element = Value

Key = ForeignKey

My $0.02.

 - nick

[James Gregory]

Appreciate the feedback guys.

I've never been big on mirroring the XML (even if we do end up doing it in a lot of cases), so I am leaning towards the rename myself and Nick mentioned; however, I'm not completely sold yet as there is some merit to sticking close to the XML for complex mappings. More opinions would be nice, but I doubt they're going to come at this point.

[Asbjørn Ulsberg]

First I want to say that I love this rewrite. Convention over
configuration always wins in my opinion, as long as the conventions
follow sensible defaults and best practices, which seems to be the
case.

Second I'd like to agree with Nick in that FNH serves its users best
when it mirrors how the programmer thinks when he designs his objects
and not how he would think if he was going to map the same object with
HBM XML. Renaming "index" to "key" and "element" to "value" makes a
lot of sense in that case. The same goes with renaming "key" to
"ForeignKey".

I haven't poked around with the code yet, but I have one question;
since the new dictionary mapping infers a lot more than previously,
how does it now handle custom collection implementations? Is that left
up to NHibernate to eventually blow up on if not configured right or
will FNH warn about it? And how would you set up a custom collection
implementation -- the same way as with HasMany?


-Asbjørn

On 4 Aug, 17:05, Nick Webb <nwe...@gmail.com> wrote:
> Fluent NHibernate is, after all, a big facade of NHibernate's xml mappings,
> no?  Being a facade, I would not concern myself with how literal the fluent
> methods resemble the hbm context, as you said it yourself: you aren't
> necessarily looking to recreate the xml in c#.  Instead, I'd follow a
> philosophy of how the methods best represent how the object property is
> being mapped.  That's my opinion, anyway.
>
> So you're example of Index -> Key and Element -> Value seems very
> appropriate, especially for those not interested in learning how to rebuild
> an hbm map the 'fluent' way.
>
> To continue this approach:
> Index = Key
> Element = Value
> Key = ForeignKey
>
> My $0.02.
>  - nick
>

> > fluent-nhibern...@googlegroups.com<fluent-nhibernate%2Bunsubscr­i...@googlegroups.com>
> > .

> > > For more options, visit this group at
> >http://groups.google.com/group/fluent-nhibernate?hl=en.
>
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Fluent NHibernate" group.
> > To post to this group, send email to fluent-n...@googlegroups.com.
> > To unsubscribe from this group, send email to

> > fluent-nhibern...@googlegroups.com<fluent-nhibernate%2Bunsubscr­i...@googlegroups.com>
> > .

[James Gregory]

I've updated the branch with basic support for custom collections, it should be able to infer a lot of the same stuff it can with regular collections; for other cases, you'll need to specify it manually.

I've also just renamed the methods to be more inline with the conceptual naming, rather than NHibernate specific.

So for anyone that's used this so far:

Key -> ForeignKey

Index -> DictionaryKey

Element -> DictionaryValue

If nobody has any objections, I'll merge this into master soon.

2010/8/10 Asbjørn Ulsberg <asbj...@gmail.com>

To unsubscribe from this group, send email to fluent-nhibern...@googlegroups.com.

[Paul Batum]

What are your plans from a versioning perspective? This is a pretty sizable breaking change. Should we start maintaining a 2.0 branch? I'm just not sure if pushing the changes directly into the current master is a good idea.

[Paul Batum]

Oops, nevermind, I see its already in there. Tally ho!

[James Gregory]

Master is 2.0, the v1.x branch is for non-breaking changes (which reminds me we need to do some merging).

[James Gregory]

To elaborate a little, I'm going to backport as many of the commits as I can to the v1.x branch and obsolete (with warnings) as much as possible. Probably not an optimal workflow... Meh

On 31 Aug 2010, at 01:33, Paul Batum <paul....@gmail.com> wrote: