Extensible Property Namers

[Tim Scott]

NOTE: I submitted this as an issue also, before this list was created.

My team has adopted NBuilder on our current project. Thanks for this
very
cool tool!!!

I like the ability to specify my own property namer. I also
appreciate
ability to extend the existing namers by overriding the virtual
method,
HandleUnknownType. However, I would like to suggest making namers
much
more extensible. To illustrate my idea, I have created the following
class:

public class ExtensibleRandomValuePropertyNamer :
RandomValuePropertyNamer
{
public IDictionary<Type, Delegate> unknownTypeHandlers = new
Dictionary<Type, Delegate>();

public ExtensibleRandomValuePropertyNamer NameWith<T>(Func<T>
handler)
{
if (unknownTypeHandlers.ContainsKey(typeof(T)))
{
unknownTypeHandlers.Remove(typeof (T));
}
unknownTypeHandlers.Add(new KeyValuePair<Type, Delegate>(typeof(T),
handler));
return this;
}

protected override void HandleUnknownType<T>(Type memberType,
MemberInfo
memberInfo, T obj)
{
var handler = GetUnknownTypeHandler(memberType);
if (handler != null)
{
SetValue(memberInfo, obj, handler.DynamicInvoke());
}
}

protected Delegate GetUnknownTypeHandler(Type memberType)
{
if (unknownTypeHandlers.ContainsKey(memberType))
{
return unknownTypeHandlers[memberType];
}
var type = GetTypeWithoutNullability(memberType);
return unknownTypeHandlers.ContainsKey(type)
? unknownTypeHandlers[type]
: null;
}

protected Type GetTypeWithoutNullability(Type type)
{
return type.IsGenericType &&
type.GetGenericTypeDefinition() == typeof(Nullable<>)
? new NullableConverter(type).UnderlyingType
: type;
}
}

I envision a property namer that uses delegates (instead of hard coded
methods) for all member types (not just unknown types). The
parameterless
c-tor of this class could initialize a dictionary with default
handlers for
most CLR types (the same ones that are now hard coded). Then the user
could swap or remove any of them. This offers fine grained control of
how
the values are set for every type. We would no longer need
ShouldIgnore;
the user can just remove handler for any type.

Here is an example of usage:

var namer = new ExtensibleRandomValuePropertyNamer()
.NameWith<Address>(AddressBuilder.Build)
.NameWith<ContactInfo>(ContactInfoBuilder.Build)
.NameWith(() => PhoneNumber.Parse(GetRandom.PhoneNumber()));
BuilderSetup.SetDefaultPropertyNamer(namer);

Using this approach we can also eliminate the need for minDate and
maxDate
fields. Something like this instead:

namer.NameWith(() => GetRandom.DateTime(new DateTime(2000, 1, 1), new
DateTime(2020, 12, 31)))

Using anonymous delegates in this way the user can exert all kinds of
control over how values are set, without the pains of inheritance.

I would be happy to work on this, but I wanted to show the idea before
making a full blown implementation.

[gd34]

Hi Tim

I really like the sound of this, great idea.

Some thoughts:

1) Does it need to be a derived class? I'd be happy for this to go on
the actual RandomValuePropertyNamer class. Or would there be some
problem with that I'm not considering?

2) I wonder if it would be possible to add this support to the
SequentialPropertyNamer as well somehow? i.e. add it to their base
class PropertyNamer so it can be used for both of them? Perhaps not
because the sequential property namer would need the current sequence
number to be passed to the delegate somehow so the method signatures
would be different.

3) I like the idea of being able to remove / override the behaviour
for individual CLR types. Again - it would be good to let the
sequential property namer have this functionality as well if it was
possible.

Gareth

[Tim Scott]

Gareth,

Good questions. I created the derived class simply to show the concept by
making it work for unhandled types. I suggest at minimum, we creating a new
class that handles naming all types using this technique. This class could
be an alternative to the existing one. In time we could deprecate the
existing one if we decide the new one's better.

If possible, yes, we should take this same approach for a sequential namer.
I have not looked at that yet. Maybe the delegates could handle the "base"
value and one other delegate could append sequence numbers for string types.
If that's feasible we should share common behaviors in a supertype.

Over the next few days, I will plan to start on this. I will ping this
group as I make progress and have questions.

Tim

[gd34]

> Good questions.  I created the derived class simply to show the concept by
> making it work for unhandled types.  I suggest at minimum, we creating a new
> class that handles naming all types using this technique.  This class could
> be an alternative to the existing one.  In time we could deprecate the
> existing one if we decide the new one's better.

[gareth] Agreed, sounds like a good plan.

> If possible, yes, we should take this same approach for a sequential namer.
> I have not looked at that yet.  Maybe the delegates could handle the "base"
> value and one other delegate could append sequence numbers for string types.
> If that's feasible we should share common behaviors in a supertype.

[gareth] Thinking about it, taking the approach of trying to do it for
both of them at the same
time might overcomplicate matters. Prob best to just start with the
random one as you've
suggested and then see if anything can be shared.

> Over the next few days, I will plan to start on this.  I will ping this
> group as I make progress and have questions.

[gareth] Cool, look forward to it.

> Tim

[Tim Scott]

I found some time this weekend and created
ExtensibleRandomValuePropertyNamer. I have attached a pathc to an issue
here: http://code.google.com/p/nbuilder/issues/detail?id=24

A few things I will point out:

1) Notice the patch contains unit tests.

2) This could certainly coexist with the existing RandomValuePropertyNamer.
However, that might be confusing to users. It could instead be a
replacement. However that is a big breaking change. I'm not sure how many
users NBuilder has yet.

3) You will notice that I added a class GetRandom, with a whole bunch of
static helper methods that wrap the random generator. I have been copying
this class from project to project for a while, and I think it would have a
nice home in NBuilder. I retrofitted it to use your random generator and
extended it to handle all of the needs of the namer class.

4) You will notice that I use GetRandom to create the default handlers in
ExtensibleRandomValuePropertyNamer. I considered whether it would be better
to inject the random generator (as the current random namer does), rather
than depend on all these static methods. However, is there really any
reason to ever swap out the implementation of the random generator,
especially now that the user has full control of how every type of member is
handled? Of course c-tor injection would make unit testing a bit more fine
grained, but is that worth the extra effort?

5) I did not spend much time thinking about whether or not we could extract
a supertype from ExtensibleRandomValuePropertyNamer that could be used to
redo SequentialPropertyNamer. Personally, I would not use the sequential
namer. However, it might be possible.

Okay, let me know what you think!

Tim Scott

[gd34]

Hi Tim

Great, I'll try it out asap - hopefully this evening.

Gareth

[garethdown44]

Hi Tim, I'm sorry it's taken so long for me to look at this.

I've just integrated the patch - I haven't looked in detail yet but
one thing that concerns me about the GetRandom class is that some of
the methods are a bit American!!

I'm British myself so things like the PhoneNumber I wouldn't be able
to use since it's an American format. Our format's quite different.
Also we don't have states or social security numbers over here.
Well... we have counties and national insurance numbers but they're
not really the same thing.

I do /definitely/ like the idea of having a PhoneNumber method though
- but perhaps these things could be grouped in some way, perhaps with
a nested class?

GetRandom.Usa.PhoneNumber()
GetRandom.Usa.SocialSecurityNumber()
GetRandom.Usa.State()


GetRandom.UK.PhoneNumber()
GetRandom.UK.NationalInsuranceNumber()
GetRandom.UK.County()

Anyway, I haven't looked through everything yet, that was just one
thing that came to mind as I was looking through the files. I'll reply
again when I've looked through properly.

Gareth

[Tim Scott]

I am way too Amero-cenntric! I like your suggestion of a nested class. Go
for it, or let me know if you want me to do it.

Tim

On 7/15/09 2:54 PM, "garethdown44" <gareth...@googlemail.com> wrote:

>
> Hi Tim, I'm sorry it's taken so long for me to look at this.
>
> I've just integrated the patch - I haven't looked in detail yet but
> one thing that concerns me about the GetRandom class is that some of
> the methods are a bit American!!
>
> I'm British myself so things like the PhoneNumber I wouldn't be able
> to use since it's an American format. Our format's quite different.
> Also we don't have states or social security numbers over here.
> Well... we have counties and national insurance numbers but they're
> not really the same thing.
>
> I do /definitely/ like the idea of having a PhoneNumber method though
> - but perhaps these things could be grouped in some way, perhaps with
> a nested class?
>
> GetRandom.Usa.PhoneNumber()

> GetRandom.Usa.SocialSecurityNumber()Go

[Tim Scott]

How goes it?

[garethdown44]

Hi Tim

Very sorry, I have a few patches people have provided that I'm trying
to integrate.

I'm hoping to finish off tomorrow evening. I'll keep you posted.

Gareth

[Tim Scott]

Gareth,

Just checking in on the status of the patch. I've applied it to a
"private" build that I am using, but that makes me feel a little
queasy. :)

Regards,
Tim Scott
Lunaverse Software

On Jul 15, 2009, at 2:54 PM, garethdown44