Possibility to extend/customise StringConverter

[nikowitt]

Hi there, a small question/improvement suggestion: it should be possible to override com.tngtech.java.junit.dataprovider.internal.convert.StringConverter, to be more precise the private Object convertValue(String str, Class<?> targetType) method. The background is to be able to support more parameter types in the @DataProvider annotation, e.g. Date and DateTime or any other types that don't have a constructor with string parameter.

Currently, the best option is to replace the StringConverter manually, but I'm sure this could be done better.

Best regards, Niko

[aaschmid]

Hi @nikowitt, yes this should be possible. I will have a deeper look at it soon. Thanks for your request!

[aaschmid]

Ok, this request is truely no problem to fulfill. The question, however, is what do you want to add? Maybe this would be a good feature to be supported directly?

[nikowitt]

Please check my pull request (https://github.com/TNG/junit-dataprovider/pull/53) for an idea that meets my requirements. In this additional method, I can provide additional string to object routines. The only idea for a feature that could be supported directly is to maybe add the support for Date/Date where a fixed date pattern could be used.

[aaschmid]

Nice, thanks for the pull request. I think, I have got your point and I really like it. Unfortunately, I am not satisfied with the solution as I think it is not easy usable by the "end user". Even if the constructor of DataConverter allows an argument, the junit-dataprovider does not use it by default. Therefore, it requires to override more classes. I would rather like some kind of listener / hook which the end user can register string custom converters to. But still not sure about it, this is just a first thought.

Have you just the String to Object conversion in mind for your feature?

Maybe we should think about a use case in form of an acceptance test and than continue to think about implementation. What do you mean?

[nikowitt]

It depends :)

In general, some additional listener is a nice idea, so of course this is a valid point. But I have to disagree regarding the simplicity of usage of my pull request because it depends on the environment that is used.

We use Spring in our environment so we already need to override the Runner as described by you in https://github.com/TNG/junit-dataprovider/wiki/Tips-and-Tricks (+some other modification we need in our environment). So this means that we already manually have to instantiate

dataConverter = new DataConverter();

in the runner. So for us, it is no additional effort to use the new constructor new DataConverter(StringConverter). And given this circumstance, it doesn't matter if a new StringConverter that extends from the existing converter is created or an additional listener is registered, the effort remains the same.

Of course this only applies to our scenario, you are perfectly right for other cases.

[nikowitt]

A type that could be implemented by default is

if (Class.class.equals(targetType)) {
            try {
                return Class.forName(str);
            } catch (Exception e) {
                throw new IllegalArgumentException("Unable to instatiate Class object from parameter '" + str + "'.", e);
            }
        } 

[aaschmid]

I like your idea about Class support for the String dataprovider that I have just implemented it (and mentioned you in the commit message :-) )

How urgent is the rest or would it be OK to think some more days about it? I get your point that you already have to override and extends some classes but this is hopefully not the normal case ... I have to think about it, though.

[nikowitt]

It is not urgent for me - I've already checked out the latest tag and patched the code with my pull request so I can use the extension ability in our environment, but of course I'd be happier when I can use the official release again :) You are right, this is not the normal case. I currently extend the StringConverter to be able to use JodaTime's DateTime objects which IMHO cannot be supported regularly due to the introduction of more library dependencies. In the future, it is planned to directly support Hibernate model classes from our application. These two requirements, JodaTime's DateTime and Hibernate, are the root of my request.

Another suggestion that doesn't have any further library dependencies is to add the support for java.util.Date objects. If you like this idea, I propose to introduce three fixed patterns which are derived from the ISO pattern:

• a DATE_ONLY pattern (yyyy-MM-dd)
• a SIMPLE_DATE_TIME pattern (yyyy-MM-dd'T'HH:mm:ss)
• a DATE_TIME pattern (yyyy-MM-dd'T'HH:mm:ss.SSSZZ).

I'm currently using DateTime.parse for this, but this could be easily backported to java.util.Date to avoid the manual string-to-date conversion which is IMHO pretty verbose in test cases.

Thanks a lot for your superb work and quick support, be prepared for some more feedback as we started to systematically introduce JGiven along with the DataProvider in our environment :)

PS: thanks for flattering me in the commit message ;)

[aaschmid]

Hehe, thanks for your feedback and your I am looking forward to more feature requests. I kind of link the idea of support for Date, though I would like to also think about it a night or more ;-) And yes, joda-time (even if I really like the library very much) is a third-party library and I would like to avoid such dependencies (even if I almost always use it ;-)).

[aaschmid]

Hi @nikowitt , can you see my suggestion, especially the integration test in dc048fb? Does that go into the right direction? Have you any other suggestions/preferences?

[nikowitt]

The integration looks perfect, that is exactly what I need :)