-
Notifications
You must be signed in to change notification settings - Fork 27
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
CECI - Implementation #21
base: main
Are you sure you want to change the base?
Conversation
Wiz Scan Summary
|
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.
Thank you for submitting the test @ceciivanov ! Can you answer the following alongside my comments?
Are you following a specific design pattern on you submission?
How do you value code comments and when do you think that they are necessary?
thanks for the effort you put in this assignment!
@@ -0,0 +1,23 @@ | |||
# Start with a base image that includes Go | |||
FROM golang:1.22.4 |
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.
Thank you for providing a containerized version here. To reduce the image size, could you consider using a multi-stage build? Are there any other techniques you might consider for minimizing the container image size?
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.
Yes, I could use multi-stage build for example builder stage for the build step and then a final stage with small base image like alpine which will copy the built artifact from the previous stage. Another technique to minimise the container image size is to remove unnecessary files, using smaller base images and using single RUN commands to reduce layers.
) | ||
|
||
// Asset interface to be implemented by all asset types | ||
type Asset interface { |
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 explain the reasoning for introducing this abstraction?
Which are the pros and cons of this approach?
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.
Using interface as an asset provides a flexible way to handle various of asset types, which can be extended in the future adding new assets. Also with the interface we can define reusable and unified operations upon them (like get details etc.) and this way we reduce the complexity of having multiple structs with many different operations.
A possible con that I can think of is that for simple applications with small number of assets and not many common and complex operations, this can add some overhead to the overall logic of the code.
} | ||
|
||
// NewInMemoryUserRepository creates a new instance of InMemoryUserRepository | ||
func NewInMemoryUserRepository() *InMemoryUserRepository { |
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.
This is a great approach. Could you explain if it would be beneficial/achievable on horizontal scaling?
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.
By trying to implement horizontal scaling with this approach I think that we will have problem with data inconsistency because each instance of application will have it's own in memory data and this could lead to synchronisation challenges & issues, for example user's favorites may differ across different instances.
To achieve horizontal scaling a good approach will be following service-oriented architecture where the application will break to micro services each handling different domain and will be independent of any other.
The approach is following the Repository/Service design which decouples the data access layer (how data is stored) from the business logic (what operations upon the data are implemented). The Repository layer provides us flexibility to implement different type of repositories for example in memory which can be used more for development and test purposes and a let's say an PostgreSQL_Repository which can be used for productive environments where the data will be stored in a database, and an easy way to swap between repository implementations without changing at all the business/application logic. The Service layer is an intermediary between the repository layer (data access layer) and the application layer. It encapsulates the business logic. The service layer can expose business logic as services or APIs that can be consumed by different clients, such as web, mobile, or third-party applications. For example if in the future we want to add a User Notification service, we can add it as a new service without worrying that it will affect any other part of the application. Code Comments are highly valued as they can enhance the readability by providing explanations that aren't obvious from the code. But I believe that good approach is not to fill up the code with comments everywhere especially when the code and what it does is pretty obvious. That's why I have included small comments for example one sentence of what each function does without adding any additional information on how this function is implemented if it's it doesn't have any complexity (for example deletion of an asset). A case where we would need more comments is if we are applying specific business rules which are not straightforward. Also when the code is more complex (for example the decoder function) it would be good to include comments on how is it implemented to avoid any misuse of it. Good practice also is to document any existing APIs with the expected inputs/outputs and behaviour. |
No description provided.