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.

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

implementation of session facade design pattern #1278 #3121

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

shahdhoss
Copy link

What problem does this PR solve?

This pull request addresses issue #1278, I implemented the Session Facade design pattern to simplify the interface for interacting with complex ecommerce functionalities. The primary goal was to demonstrate how the Session Facade can act as an intermediary, providing a simplified interface to a series of interconnected, complex operations. For more info please refer to the readme file under the session-facade module.

Comment on lines 40 to 44
<properties>
<maven.compiler.source>17</maven.compiler.source>
<maven.compiler.target>17</maven.compiler.target>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>
Copy link
Owner

Choose a reason for hiding this comment

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

These come from the parent pom.xml

<scope>test</scope>
</dependency>
</dependencies>

Copy link
Owner

Choose a reason for hiding this comment

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

Add maven-assembly-plugin so that we can execute the code example from the command line. See other patterns for examples.

Comment on lines 28 to 30
/**
* The type App.
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Explain the pattern here. Describe how the code example implements it.

Comment on lines 74 to 81
for (Product product : productCatalog) {
if (productId == product.id()) {
this.cart.remove(product);
LOGGER.info("{} successfully removed from the cart", product);
return;
}
}
LOGGER.info("No product is found with the id {}", productId);
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of iterating list, could we use a more convenient data structure (e.g. map)?

@shahdhoss
Copy link
Author

Thank you very much for your valuable feedback, I have reviewed the comments and made the necessary changes.

@shahdhoss shahdhoss requested a review from iluwatar December 20, 2024 12:14
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