Skip to content

[Java] Convert title case headers to property names #1751

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 11 commits into from
Sep 2, 2019
Merged

Conversation

rasklaad
Copy link

@rasklaad rasklaad commented Aug 26, 2019

Summary

User can convert data table title case header, using default data table transformer

Motivation and Context

#1746

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@coveralls
Copy link

coveralls commented Aug 26, 2019

Coverage Status

Coverage increased (+0.03%) to 88.041% when pulling e4494cc on issue-1746 into ce9b61c on master.

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

I don't think extending JavaDefaultDataTableEntryTransformerDefinition with a JavaDefaultDataTableCapitalCaseTransformerDefinition is the right solution though.

Inheritance should only be used when there is a clear 'is a' relationship between classes. In this case this relation ship is not so clear to me. They are both obviously DefaultDataTableEntryTransformerDefinitions but the functionality of JavaDefaultDataTableCapitalCaseTransformerDefinition is a strict subset of JavaDefaultDataTableCapitalCaseTransformerDefinitions functionality. In these scenarios is better to prefer composition over inheritance and use a decorator pattern.

You could implement a DefaultDataTableEntryTransformerDefinition that delegates all calls to another instance of DefaultDataTableEntryTransformerDefinition (in this case always a JavaDefaultDataTableCapitalCaseTransformerDefinition) and decorates the execute call.

However when you open up a class for extension, or compose it in another class you also have to write tests to see if the extension/composition behaves in the same way the parent class does. This generally makes for some complicated test suits.

And given the simplicity of it all I don't think that either composition or inheritance is worth the structural cost. Adding a boolean as field to JavaDefaultDataTableEntryTransformerDefinition should be sufficient.

@mpkorstanje mpkorstanje changed the title Data table title case headers transformer [Java] Convert title case headers to property names Aug 31, 2019
@mpkorstanje
Copy link
Contributor

Added an exception to deal with collisions when converting headers and moved the camel case conversion to core so the different backends don't have to deal with individually.

@mpkorstanje
Copy link
Contributor

And with that I think we're done. @rasklaad is there anything you'd think we should add?

@rasklaad
Copy link
Author

rasklaad commented Sep 2, 2019

Nah, I think we are fine

@mpkorstanje mpkorstanje merged commit a2270fc into master Sep 2, 2019
@mpkorstanje mpkorstanje deleted the issue-1746 branch September 2, 2019 19:07
@mpkorstanje
Copy link
Contributor

Cool. Cheers. Much appreciated!

@gurpiarbassi
Copy link

@mpkorstanje I've been trying to piece together what needs to be done in order to make use of this lovely functionality. I've got a table with columns in title case e.g. "Order Id", "Customer Id"

I've got a object like:

Order {
 String orderId;
 String customerId;
}

I've got a step definition

which takes Order as an argument.

Do I need to register default table transformer or should it work out the box?

@mpkorstanje
Copy link
Contributor

Do I need to register default table transformer

Yes. Using the annotation.

@gurpiarbassi
Copy link

sorry, may sound like a stupid question but where do you register this?
I'm new to cucumber and only just learnt about registration by implementing TypeRegistryConfigurer.

Are you saying the step needs to be annotated?

(By the way it's a Java 8 style step)

@gurpiarbassi
Copy link

It's alright. I worked it out.
Should have read the readme thoroughly.

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

Successfully merging this pull request may close these issues.

4 participants