Skip to content

Allow to override default types with length or precision parameters #2100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 11, 2019

Conversation

bahusoid
Copy link
Member

@bahusoid bahusoid commented Apr 3, 2019

It's possible since #702 to override default type with custom type. But current solution is limited as it doesn't handle precision or length based types.

So this PR aims to fix this limitation and it allows:

  1. Transparently replace any default type with custom type (with exception for places where explicitly NHibernateUtil.SomeType is used like IQuery set param methods SetString, SetDateTime,...)
  2. Simplify custom type mapping by reusing existing length, precision and scale mapping attributes.

Example of string override:

//Somewhere before SessionFactory is created
TypeFactory.RegisterType(
	typeof(string),
	new CustomStringType(),
	new[] {"string"},
	configurator => configurator.SetTypeByLengthOrScale(length => new CustomStringType(length)));

@hazzik
Copy link
Member

hazzik commented Apr 15, 2019

Why not just like this?

//Somewhere before SessionFactory is created
TypeFactory.RegisterType(
	typeof(string),
	new CustomStringType(),
	new[] {"string"},
	length => new CustomStringType(length));
//Somewhere before SessionFactory is created
TypeFactory.RegisterType(
	typeof(string),
	new CustomStringType(),
	new[] {"string"},
	(scale, precision) => new CustomStringType(scale, precision));

Why do we need TypeConfigurator?

@bahusoid
Copy link
Member Author

To avoid public methods multiplications. In 6.0 it's going to be single public method which looks cleaner to me. Also it simplifies further extension - you can add any number of members to TypeConfigurator without breaking changes and still have only single public method exposed.

Well... If you are not convinced can make two additional methods as you suggested.

@hazzik
Copy link
Member

hazzik commented May 5, 2019

and still have only single public method exposed.

I don't mind having a method overload.

@bahusoid
Copy link
Member Author

bahusoid commented May 6, 2019

I don't mind having a method overload.

Overloads with delgates tend to collide so I prefer avoid such overloads if possible. For instance length => new CustomStringType(length) now is nicely resolved by compiler to GetNullableTypeWithLengthOrScale. But if you want to add some similar delgate with different input parameter like NullableTypeCreatorDelegate - you have to use new method name as overload will be ambigious for lambda calls.

So are you sure with overloads?

@hazzik hazzik merged commit 4d0da00 into nhibernate:master Jul 11, 2019
@hazzik hazzik added this to the 5.3 milestone Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants