-
Notifications
You must be signed in to change notification settings - Fork 796
Replaced tuples with domain names #32
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
Conversation
|
Thanks! |
|
Hi, @krzykrucz - any plans on this minor remarks? :) |
|
Hi @pilloPl, have you made any comments on the changes? I don't see any... |
| private EntityModel<Checkout> resourceWithLinkToCheckoutSelf(UUID patronId, Tuple2<BookId, Instant> checkout) { | ||
| return new EntityModel<>(new Checkout(checkout._1.getBookId(), checkout._2), linkTo(methodOn(PatronProfileController.class).findCheckout(patronId, checkout._1.getBookId())).withSelfRel()); | ||
| @Value | ||
| private class Hold { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a reason for Hold and Checkout to be a inner class with a reference to an outer class :) can we revert it?
| import io.pillopl.library.lending.patron.model.PatronId; | ||
| import io.pillopl.library.lending.patronprofile.model.PatronProfiles; | ||
| import io.vavr.Predicates; | ||
| import io.vavr.Tuple2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert changes in imports? We decided not to use * in imports
| @@ -1,17 +1,13 @@ | |||
| package io.pillopl.library.lending.patronprofile.model; | |||
|
|
|||
| import io.pillopl.library.catalogue.BookId; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert changes in imports? In all of the files?
|
Wow, I have commented but have not submitted. Facepalm. So sorry :) |
|
No problem 😄 I'll make corrections |
pilloPl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
@krzykrucz seems like this broke one test: shouldContainPatronProfileResourceWithCorrectHeadersAndLinksToCheckoutsAndHolds - io.pillopl.library.lending.patronprofile.web.PatronProfileControllerIT Unfortunately I cannot investigate it right now |
|
@pilloPl I'll take a look in the evening |
|
Any progress with that? |
|
I've just had a look at it. I could reproduce the error only by running the tests in maven (in IntelliJ they were green). And it turns out, I get this error even before my changes. |
I replaced tuples in domain models with domain types, because tuples are names not included in a ubiquitous language.
In my opinion, tuples should only be used as a business workflow output, when the objects returned can't be grouped together as another business object.
I'd be glad with any comments you'll have regarding the names I chose.