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.

Sign up for GitHub

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

Jump to bottom

Improve naming of find(…) methods in OrderManager #61

Closed
odrotbohm opened this issue Nov 29, 2014 · 5 comments
Closed

Improve naming of find(…) methods in OrderManager #61

odrotbohm opened this issue Nov 29, 2014 · 5 comments
Assignees
@odrotbohm
Labels
type: enhancement Improvements and new features
Milestone
6.2 GA

Comments

@odrotbohm
Copy link
Member

No description provided.

@odrotbohm odrotbohm self-assigned this Nov 29, 2014
@odrotbohm odrotbohm added this to the 6.2 milestone Nov 29, 2014
@AndreasZaschka
Copy link

i can do this

@odrotbohm
Copy link
Member Author

Feel free to go ahead. Be aware that we need to deprecate the existing methods to not break user code and provide a migration path.

@odrotbohm odrotbohm changed the title Rename UserAccount.find(…) methods to findOrdersBetween(…) etc. Rename OrderManager.find(…) methods to findOrdersBetween(…) etc. Dec 9, 2014
@AndreasZaschka
Copy link

ok, i take care of that.

Is it part of M 6.2 or 6.x

@odrotbohm
Copy link
Member Author

I don't suspect that to be done until the end of the week, which is when 6.1 is scheduled for. Please create a pull request against master and we can then decide which branches to merge it to.

@odrotbohm
Copy link
Member Author

@AndreasZaschka - The commit you linked looks like something we could actually use. Mind to create a pull request from that one?

@AndreasZaschka AndreasZaschka added the lifecycle: in-progress Being worked on… label Aug 26, 2015
@odrotbohm odrotbohm modified the milestones: 6.2 GA, 6.2 RC1 Oct 9, 2015
@AndreasZaschka AndreasZaschka added the type: enhancement Improvements and new features label Oct 19, 2015
odrotbohm pushed a commit that referenced this issue Oct 20, 2015
@odrotbohm
#61 - Rename OrderManager.find(…) methods to findOrdersBetween(…) etc.
3623bb6 
Original pull request: #106.
odrotbohm added a commit that referenced this issue Oct 20, 2015
@odrotbohm
#61 - More consistent finder names in OrderManager.
35a4b3e 
Slightly tweaked the method names in OrderManager to make sure calling them doesn't create unnecessary duplication in terms. E.g. a previously existing call:

Iterable<Order> orders = orderManager.findOrdersByUserAccount(userAccount);

can now be written like this:

Iterable<Order> orders = orderManager.findBy(userAccount);

The types and variable names carry enough information to communicate what's  going on. Removed final declarations for methods in PersistentOrderManager as the type is package protected anyway.

Introduced a builder style factory method for Intervals so that creating them changes from new Interval(start, end) to a more fluent Inveral.from(start).to(end). This also allows us to remove some assertions from the OrderManager implementation into the value object which makes them much more testable. Adapted test cases according to the API changes.

Original pull request: #106.
@odrotbohm odrotbohm changed the title Rename OrderManager.find(…) methods to findOrdersBetween(…) etc. Improve naming of find(…) methods in OrderManager Oct 20, 2015
@odrotbohm odrotbohm removed the lifecycle: in-progress Being worked on… label Oct 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@odrotbohm odrotbohm

Labels
type: enhancement Improvements and new features
Projects
None yet
Milestone
6.2 GA
Development

No branches or pull requests
2 participants
@odrotbohm @AndreasZaschka