Skip to content

#348 Data Transfer Object design pattern#608

Merged
iluwatar merged 12 commits into
iluwatar:masterfrom
gopinath-langote:master
Aug 17, 2017
Merged

#348 Data Transfer Object design pattern#608
iluwatar merged 12 commits into
iluwatar:masterfrom
gopinath-langote:master

Conversation

@gopinath-langote
Copy link
Copy Markdown
Contributor

Basic version of Data transfer object pattern implementation.

Copy link
Copy Markdown
Owner

@iluwatar iluwatar left a comment

Choose a reason for hiding this comment

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

The project file for the class diagram is missing (.ucls)

Comment thread data-bus/pom.xml Outdated
<packaging>pom</packaging>
<modules>
<module>../data-transfer-object</module>
</modules>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why is Data Bus pattern affected? Should this change be reverted?

Copy link
Copy Markdown
Contributor Author

@gopinath-langote gopinath-langote Aug 13, 2017

Choose a reason for hiding this comment

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

@iluwatar 👍 . I reverted back. Thanks

customers.add(customerOne);
customers.add(customerTwo);

CustomerResource customerResource = new CustomerResource(customers);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is customer server data initialization, right? Could we move to static block in CustomerResource class?

Copy link
Copy Markdown
Contributor Author

@gopinath-langote gopinath-langote Aug 13, 2017

Choose a reason for hiding this comment

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

@iluwatar Yes, we can. But the unit tests for the CustomerResource class will not be pure. As the class itself is having some initial data. Don't you think the unit tests will be more confusing one?

}

private static void printCustomerDetails(List<CustomerDto> allCustomers) {
allCustomers.forEach(customer -> System.out.println(customer.getFirstName()));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't use System.println, instead add and use a logger as in other examples.

Copy link
Copy Markdown
Contributor Author

@gopinath-langote gopinath-langote Aug 13, 2017

Choose a reason for hiding this comment

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

@iluwatar 👍 Ok. Make sense. Did the same.
Thanks.

public class CustomerDto {
private String id;
private String firstName;
private String lastName;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This looks like immutable class. The private members should be declared final.

Copy link
Copy Markdown
Contributor Author

@gopinath-langote gopinath-langote Aug 13, 2017

Choose a reason for hiding this comment

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

@iluwatar : Done. Thanks.

@gopinath-langote
Copy link
Copy Markdown
Contributor Author

@iluwatar : Added missing class diagram.

Thanks.

@iluwatar
Copy link
Copy Markdown
Owner

@gopinath-langote looks good now. Thanks for the contribution! 👍

@iluwatar iluwatar merged commit e367e48 into iluwatar:master Aug 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants