six2six / fixture-factory

Generator fake objects from a template
Apache License 2.0
445 stars 88 forks source link

Jodatime support #80

Open ismaelgcosta opened 8 years ago

ismaelgcosta commented 8 years ago

I need a custom rule for JodaTime fields like Datetime, LocalDate, etc. Can you add this function? Or can I add this function on the project? If are not possible to do in that project, you can create a fixture-factory-extensions, with support for external Java libraries.

Thank you.

nykolaslima commented 8 years ago

Since not everyone is using JodaTime, maybe it would be nice to think in a fixture-factory-extensions. But it would be easier to add this support in the core and make JodaTime dependency optional. @aparra @ahirata what do you guys think about it?

maurcarvalho commented 8 years ago

Hello guys, I watch this project for a while and actually I used it in a production environment. Maybe it's my opportunity to contribute, follow my 2 cents. Once Java 8 (and next versions) is using JodaTime as a default date/time library sounds like a good ideia to support it by default on Fixture-Factory project. Like I've said before I can help with this branch in order to make it happens.

2015-11-18 15:56 GMT-02:00 Nykolas Laurentino de Lima < notifications@github.com>:

Since not everyone is using JodaTime, maybe it would be nice to think in a fixture-factory-extensions. But it would be easier to add this support in the core and make JodaTime dependency optional. @aparra https://github.com/aparra @ahirata https://github.com/ahirata what do you guys think about it?

— Reply to this email directly or view it on GitHub https://github.com/six2six/fixture-factory/issues/80#issuecomment-157801647 .

ismaelgcosta commented 8 years ago

Like a fixture-factory-jdk8 version? I would like to see this (and to contribute also)

nykolaslima commented 8 years ago

@MauricioJr It would be great. I'm using f-f with JDK 8 and I don't have any problems yet. But support the new date api would be awesome.

Please fell free to go ahead and implement it, this way we can launch a new version supporting java8's date api :+1:

ismaelgcosta commented 8 years ago

Hi, I forked the project, end started to add support java8's date.

It is still raw, but I would like to hear from you before proceeding. The names may sound a little confusing because the new date has instant class (such as the current method to date).

My solution is not so good I think, if you have suggentions, I would appreciate to listen.

Thanks.

https://github.com/mundodojava/fixture-factory

ahirata commented 8 years ago

@mundodojava hey man, maybe I'm missing something but, as far as I can tell, you don't need to create all those functions.

Isn't it just a matter of converting the Calendar to the java.time types? If so, you could just change the CalendarTransformer or create a new Transformer pretty much like the DateTimeTransformer you created.

Also, can you make sure you commit only the lines you have actually changed? Your commits are changing the whole file which makes a bit hard to review and keep track of the modifications.

aparra commented 8 years ago

Hey @mundodojava,

There is a corner case in convert Calendar to LocalDate using this approach: LocalDate.of(cal.get(Calendar.YEAR), cal.get(Calendar.MONTH), cal.get(Calendar.DAY_OF_MONTH))

Month in Calendar is zero-based, so if convert today LocalDate to today Calendar you will receive one month ago

Instead of plus one in the month, you could use it: calendar.toInstant().atZone(ZoneId.systemDefault()).toLocalDate()

Please, check your test. The Calendar.getInstance() method returns now but you are changing the date for the past. And finally, check the instructions from @ahirata. :)

ismaelgcosta commented 8 years ago

Ok, I will do this changes. As soon as possible, and thank you for the instructions

ismaelgcosta commented 8 years ago

I create a new pull request here.

https://github.com/six2six/fixture-factory/pull/81

maurcarvalho commented 8 years ago

@douglasrodrigo could you please close this issue too? Related Pull Request -> #81