Re: [morph-developer] InstantiatingReflector

[Ben A. <ben...@ac...>]

Matt Benson wrote:
> Ben, can you provide a little more info on your setup?
>   

I'd love to. Most of the world are developing anemic domain objects, so
public no-argument constructors in so-called "domain objects" are
everywhere. There's no question as to their convenience, but there is
significant momentum behind a shift to domain driven design, which
essentially advocates proper object orientation - including both
encapsulation and state integrity. As such, people are actually starting
to use constructors for their original purpose, being to construct an
object and then allow the object's methods to continue to preserve state
integrity.

In ROO we use constructors typically to represent the mandatory
properties of an object, such that the object cannot exist in memory
unless those mandatory properties are provided. We limit the provision
of mutator methods to only those properties which are not state managed
(ie we don't care what they become) or the mutator method itself is
capable of ensuring validation.

We are using Morph to covert between data transfer objects and domain
objects. As such, an input DTO will contain all of the properties
necessary to construct a domain object. However, we need to map between
the DTO fields and the domain object constructor parameter names. It's
quite easy to extract this information, and Spring offers an interface
(ParameterNameDiscoverer) for this purpose. I'll describe it further below.

In terms of my change request, I would suggest implementation as follows
(I've done this on a local copy and it worked well):

- Change the existing InstantiatingReflector.newInstance(Class) method
to include a second "Object parameters" parameter.
- Change the final BaseReflector.newInstance(Class) method to final
BaseReflector.newInstance(Class, Object). Delegate through to a new
method, BaseReflector.newInstanceImpl(Class, Object).
- Change the BaseReflector.newInstanceImpl(Class) to be marked as final
and deprecated, noting that subclasses should now use
BaseReflector.newInstanceImpl(Class, Object). Marking it as final will
of course immediately show up any of the deprecated users of the method
as people upgrade Morph.
- Add a new BaseReflector.newInstanceImpl(Class, Object) method which by
default has the same behavior as the old
BaseReflector.newInstanceImpl(Class).
- Modify Morph subclasses accordingly to no longer use the
BaseReflector.newInstanceImpl(Class) method. Such classes are easy to
find, as the superclass method is now marked final.
- Modify BaseTransformer.createNewInsanceImpl(Class, Object source) to
delegate the source argument through to the InstantiatingReflector method.

Realistically, I don't anticipate many end users (ie outside Morph
framework itself) would have overridden
BaseReflector.newInstanceImpl(Class), simply because there isn't a great
deal that you can do with that method signature in the first place. I
mean, you can create the required object using its no-argument parameter
and return it. If people are doing more exotic things, they'd probably
like the flexibility of the new signature format. You know the Morph
community better than I do, but I don't think this simple change (adding
one parameter to a method signature if they happen to override
BaseReflector.newInstanceImpl(Class)) is going to be necessary for many
people.

As you may have guessed, I don't believe that an additional method on
the InstantiatingReflector interface is ideal. I did suggest it, as a
way of avoiding backward compatibility issues, but the legacy of the
decision will remain far longer than the deprecated final
BaseReflector.newInstanceImpl(Class) needs to be retained. You'd also
need users of InstantiatingReflector to decide which method to invoke,
and/or maintain separate delegation approaches in common superclasses
like BaseReflector. It just doesn't feel as clean as marking a method
final and deprecated, and being done with a nice clean change.

To explore Matt B's suggestion as to an automatic algorithm to construct
the object if it doesn't offer a no-argument constructor, what we
currently do is look for the shortest public constructor on the
destination class and use it. If it's no-argument, that's easy. If it
accepts arguments, obtain the parameter names from the compiled source
code's debug information via Spring's ParameterNameDiscoverer and then
look for accessor methods or public fields on the source object that
match those names. If found, you have an argument to present as that
parameter. If not found, configurable behaviour should be offered such
as throwing an exception, trying the next public constructor, or using
null (or a suitable default if a primitive). Such behaviour would appear
quite a comprehensive approach for something like
BaseReflector.newInstanceImpl(Class, Object), although I am not
necessarily advocating its inclusion. An issue to contemplate is the
need to include an ASM dependency for the necessary parameter name
visitor support, and possibly Spring if you'd like
ParameterNameDiscoverer and a usable implementation. There's something
to be said for keeping Morph lightweight and free of additional
dependencies, leaving more exotic users to add those JARs and override
BaseReflector (I am happy to do this). Equally, this logic might be
positive to include, instead using reflection to obtain the method from
ParameterNameDiscoverer if it's detected in the classpath (thus avoiding
a compile-time dependency but still offering more sophisticated
construction support for the many people who have Spring in their
classpath).

If someone can make a decision on the approach, I am very happy to
implement it and provide the patch (I can also provide what I've already
done, as described above).

Cheers
Ben