First, some quick impressions: - Its conceptually nice to relegate XML mapping to a less-central role within NH (e.g., XML now becomes just ONE of MANY possible 'mapping input syntax forms') so I applaud the effort/intent - Decoupling mapping + config from XML entirely will definitely support many more alternate declarative inputs (custom DSL, whatever) in a manner much more flexible than having to depend on XML as an 'intermediate translation layer' for NH to consume mapping/config info - I like that the thrust of the syntax more closely parallels the nodes/attributes used in the XML files, making it approachable to existing users of XML mapping files (reasonably shallow learning curve) even as this leads to a fairly verbose API (compared let's say, to something like FNH where 'terse API' was --from my reading-- an implicit design goal)
Next, some more specific feedback/issues/thoughts/concerns we might want to consider (in no particular order): - If this API is to become 'authoritative' for mapping, then by extension it clearly needs to completely support all existing 'xml constructs' fully - Unlike FNH where the authors have taken the entirely valid approach of "we're releasing with support for the 80% of NH mapping constructs that are most-commonly in use, permitting people to weave in xml-based content for the parts we don't yet support in code, and over time we will evolve to support the remaining 20% edge-cases in the fluent API", IMO for NH this API shouldn't be released into the codebase until it has 100% functional equivalency with each and every existing XML construct and doesn't rely on intermingling xml for any of its needs. - Obviously as an existing proof-of-concept this isn't yet done but it would seem important before making this the authoritative API to ensure that adequate spikes are done to be certain that for every 'statement' presently possible in XML, there's at least an idea about how to express that in this new API (some of the edge-cases that come to my mind initially are things like the <database-object> elements, stored procedures, filter statements, where statements, etc. that aren't immediately obvious from the sample code in the blog post re: how they would be addressed -- basically the entire category of things presently definable in XML that aren't necessarily *directly* about mapping entities per-se but that are about caching directives, other DB elements, etc.) - We need to ensure that adequate consideration is given to how this API would express things that aren't immediately possible to state as lambda's without some 'tricks' via reflection, etc. For example, this kind of statement from the sample code... map.Class<Animal, long>(animal => animal.Id, id => id.Generator = Generators.Native, rc => ...won't work where the POID is actually not a public property of the Animal class but is instead a private field and of course the statement (animal => animal._persistentId, ... won't compile b/c the private field _persistentId is not visible to intellisense (or more importantly, the compiler!). To be sure, there are 'tricks' to get around this lambda limitation, but I recommend that we should consider carefully what this will look like when invoked via this API to be sure its as clear as the public-property-based mappings shown in the sample code. - How does moving this fluent-API into the role of 'primary' mapping API affect NH's 'parity' with Hibernate? Is it possible that Hibernate might get a future feature that would want to be ported to NH but we would find that this (hypothetical) new feature isn't expressible in the new fluent API? Obviously nobody can predict the future, but the move to this API as 'primary' mapping API would seem to represent a digression from NH's close parity with H which may impact the ease with which future features can be ported from H to NH. This new fluent API might very well be a useful/valuable digression (and obviously there are already OTHER features that have been added to NH that take advantage of the .NET platform and don't have an allegory in Hibernate so diverting from exact parity with H isn't without precedent), but IMO we should carefully consider its potential future impact on porting H features to NH before we decide to 'deprecate' XML as the primary/core mapping API. In short: I like the approach, think it will be quite valuable and helpful to anyone trying to write inputs to the NH mapping/config API in the future, but recommend that we consider the above issues/thoughts before introducing this API as a 'replacement' for the primacy of the xml API in the project. HTH, Steve Bohlen [email protected] http://blog.unhandled-exceptions.com http://twitter.com/sbohlen On Sun, Jan 17, 2010 at 5:40 PM, Fabio Maulo <[email protected]> wrote: > Please, I'm needing feed-back coming from code-review. > > Try to think at it as a possible mapping-by-code solution for NH3.0. > I'm seriously thinking to add it in NH directly. > > 2010/1/13 Fabio Maulo <[email protected]> > >> Hi team. >> >> I would like to have a code re-view of my last post and >> a constructive feedback >> http://fabiomaulo.blogspot.com/2010/01/map-nhibernate-using-your-api.html >> >> >> <http://fabiomaulo.blogspot.com/2010/01/map-nhibernate-using-your-api.html>The >> post, and overall the code, is basically an example about how implement a >> custom API to create mappings by code. >> That is an invitation to everybody want create his own API and even an >> invitation for FNH team to use the Hbm* classes instead generate XML. >> >> That said, seeing how things are going in each framework, NH needs its own >> mapping-by-code. >> Two matters here: >> 1) API definition >> 2) usage of Hbm* or directly create metadata (classes of namespace >> NHibernate.Mapping) >> >> What is clear is that our implementation will supports anything supported >> by XML and, probably, we will improve some of actual >> "conventions-interceptors" (as, for instance, INamingStrategy & Co. ). >> >> -- >> Fabio Maulo >> >> > > > -- > Fabio Maulo > >
