Skip to content
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

Timestamp or Date value Editors are not working, if xls data is of Date/time type #47

Open
ravi-polampelli opened this issue Oct 12, 2012 · 5 comments

Comments

@ravi-polampelli
Copy link

Hi Anuj,

When I was trying to test I find issue with handling java.sql.Timestamp params. I think issue is same with Date or Time values. I have tried to create Timestamp Editor, but Editor setAsText takes String parameters, where as in XLS user might enter any type of data in cell, in this example I have entered Date type in cell. But it is throwing conversion problems like java.util.Date can not be converted to String.

I think we should have some generic way of handling such things. Below is my plan, please let me know if you see any issues with it.

  1. Create a GeneralUtil class, which will have methods like convertToSQLTimestamp(Object object), convertToSQLTime, convertToSQLDate, convertToDate, convertToInteger, Double, Float, Long etc..
  2. Inside those methods we can check instance of Object, and if it is instance of any other type than expected, then we can use wrappers, values. parse etc.. and convert it to expected type
  3. Update Param annotaion class, if Editor or Converter is not found, and Param class is of above type (Date,Time,Timestamp, Long etc...) then use GeneralUtil.convertTo....methods.

Advantages:

  1. We are abstracting all conversion logic inside easy test for known data types(non user defined). so user is not worried to write any Editors or Converters for them.

Please let me know how it sounds.

Cheers,
Ravi

@anujgandharv
Copy link
Member

Hi Ravi,
I think its a good idea to abstract the date, timestamp values into a separate util class so that the user does not have to write converters for such datatypes. For data types like Long, Integer etc theres already a generic conversion in place and so I dont think we have to write separate methods for them. But I agree it would be a good idea to do it for Date, Time, Timestamp etc. One thing we have to note is that we should still give users the ability to provide their own converters and that should have priority over our mechanism. Also another thing to note is that we should be able to handle date values in XML and CSV as well(which I think should not be a major problem).
I see you volunteering for this work as you have done most of the investigation in it. :).

Cheers!

@ravi-polampelli
Copy link
Author

For Long,Integers etc.. generic PropertyEditors exist and they take String values as input, Hence I think they are not sufficient as test data may come in any other format. Lets say in case of Excel the cell type can be modified by user, if user changes type other than String then those property editiors will fail.
Hence, I think its better to give priority to Converters, then this util methods, then to PropertyEditors.. what do you say?

@anujgandharv
Copy link
Member

Right. Got your point. And I think you have an excellent point here. Let's create a util class that can handle Excel data as you mentioned above.
Good pointers. I am collecting all these pointers and I will be mentioning it on 31st October in my presentation, along with your name to a wider audience. The session will be recorded(hopefully). I will send you the link.

Cheers!

@ravi-polampelli
Copy link
Author

Hi Anuj,

Its good to know about 31-Oct presentation.
I have created Util class, i will upload that soon. I couldn't merge pull request, I am not sure why?

Cheers,
Ravi

@anujgandharv
Copy link
Member

Hi Ravi,
I think its because of the ExcelDataLoader changes. There are so many changes that have gone in to master since you last merged with master. Thus I would suggest try a manual merge. Take a look at this guide on how you can do a local merge. It should be able to help you do the merge easily.: http://nathanj.github.com/gitguide/tour.html
Let me know if you face any iussues and we can then try to resolve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants