-
-
Notifications
You must be signed in to change notification settings - Fork 27k
Hexagonal blog #487
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
Hexagonal blog #487
Conversation
So i havent read your blogpost from start to finish 100%, as this isnt my style in general, i always scroll back and forth to scan for relevant information in posts and only want to read the good information. At the very beginning the explanation is good, once read through, without much skipping i got the general idea of the pattern. 👍 Some code parts are also included that i think of unnecessary, like the LotteryTicket#hashCode #equals, getter methods and imports. I dont think the reader needs those informations right at the moment he/she comes in first contact with the new pattern (well, i didnt at least). The page-filling amount of code breaks the "flow" for me, its like that for me: For me the blogpost is still valueable, as the code often does contain good commenting and explanation. I just think there has to be a better way of representing the informations and comments in different way than just Dont get discouraged by me! I really ❤️ the idea of a blogpost explaining the pattern in detail and i think this is what jdp really needs at this point. |
@markusmo3 thanks for the review and valuable comments. I'll try to cut down the amount of code, especially the irrelevant parts. I understand it can be disheartening for the reader to have to read hundreds of lines of code at once. |
My doubt regarding lottery system was not regarding the name. Should lottery system be cled a port? And if yes then what will be different adapters of this port? |
@npathai I won't publish this weekend so you have time to suggest the new image. Can you elaborate a bit more the idea of lottery system port? How would you change the system? |
@iluwatar There you go. Not exactly my idea, saw it being used somewhere, found a creative commons image and edited it to add blogpost title. Hope you like it. |
@iluwatar My concern in the present explanation is that the core of the system does not contain any business logic, there are only data structures like One of the reason hexagonal architecture is utilized is ease of testing the system without dependency on database or such, but in our system there will be nothing to test but the datastructures. This feels a little weird to me. Shouldn't business logic of how ticket numbers are selected be part of the core domain and not a port? Any insights would be great. Thanks 👍 |
@npathai Yes, I tend to agree with you comment that the core should include domain logic. I think I will try to add another class I like the new image and will try to use that instead of the controversial nut 😁 |
@npathai The work is ready for another review. I've made the changes as described in the previous comment. @markusmo3 I understood the feature concerning |
<script src="http://gist-it.appspot.com/http://github.com/iluwatar/java-design-patterns/raw/master/hexagonal/src/main/java/com/iluwatar/hexagonal/notifications/LotteryNotifications.java?slice=26:"></script> | ||
<script src="http://gist-it.appspot.com/http://github.com/iluwatar/java-design-patterns/raw/master/hexagonal/src/main/java/com/iluwatar/hexagonal/notifications/LotteryNotificationsImpl.java?slice=26:"></script> | ||
|
||
The methods in `LotteryNotificationsImpl` adapter are simple `System.out` printers so the implementation is trivial to understand. |
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.
A better name for LotteryNotificationsImpl
would be ConsoleNotifications
. Also if the interface represents an entity that will send out notifications then the name can be something like LotteryEventNotifier
, and the console implementation can have name ConsoleNotifier
and maybe a SMSNotifier
.
Also in the code there are *Impl
classes, I am not fond of names like *Impl
, rather the implementation should describe how it does, what it does. Plus why do classes like LotterySystemImpl
hardcode their dependency on LotteryNotificationsImpl
, LotteryTicketInMemoryRepository
, WireTransfersImpl
. Out of these lottery ticket repository and Wire Transfers are dependencies of the system and should be injected using constructor. What if we wanted to test the system using MongoTicketRepository
?
@iluwatar Sure. Have a look at the last comment here: #486
Such include parameters for external use are always documented in the include file itself Due to jekyll not "really" being a programming language some stuff might sound a bit wierd, but those includes/imports could also be imagined as a method/function, taking parameter and returning html code. Hope this clears it up a bit. EDIT: The hash thing might be something to easily miss. Pseudo Code:
Reminder: a page variable is the things in the front matter (*1): 732774f Further reading: https://jekyllrb.com/docs/templates/#includes |
@iluwatar Sure I will review and update the diagram on this weekend. |
@iluwatar Here is the updated diagram |
@iluwatar I am done with review. It seems good to go. Great job done on this blog. 👍 |
@npathai @markusmo3 thank you very much for the help! The blog post just went live. |
@all-contributors please add @markusmo3 for design code ideas |
I've put up a pull request to add @markusmo3! 🎉 |
This is a pull request for a blog post about Hexagonal Architecture. The code samples have been embedded using gist-it. Looking forward to your comments.