Skip to content

Conversation

@MiisaelCabreraEnc
Copy link
Owner

No description provided.

@MiisaelCabreraEnc MiisaelCabreraEnc self-assigned this Dec 20, 2024
Copy link
Collaborator

@iuresti iuresti left a comment

Choose a reason for hiding this comment

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

Excelent first try!

Remember in order to merge your PR all your code must be covered by unit tests.

One more recommendation: Add interfaces for your service and your repository.. so your controller depends on the interface of the service and the service depends on the interface of the repository, not on the implementations

backend/pom.xml Outdated
<name>Backend</name>
<description>Demo project for Spring Boot</description>
<url />
<licenses>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend to remove the entries that you're not using: url, licenses, developers, scm

public class BackendApplication {

public static void main(String[] args) {
SpringApplication.run(BackendApplication.class, args);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Review the settings of your IDE, it's using tabs for identation, usually we use 2 or 3 spaces

import org.springframework.web.bind.annotation.RestController;

@RestController
public class HelloWorldController {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend to remove this class as it was just for testing and shouldn't belong to your project.

@RequestMapping("/products")
public class ProductController {

ProductService productService = new ProductService();
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't forget the access modifier (private).

You must not instantiate your ProductService by yourself, spring boot manage the instances of your beans and will inject them wherever you require it

}

@PostMapping("/{id}/outofstock")
public Product outOfStock(@PathVariable Long id) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this method do? .. The name is not descriptive at all (same for inStock)

return null;
}

public Product updateStock(Long id, int stock) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

update stock is business logic, that logic belongs to a service layer not repository layer.


import java.util.Optional;

public class ProductService {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Annotate this class with @Service

import java.util.Optional;

public class ProductService {
private ProductRepository productRepository = new ProductRepository();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't instantiate ProductRepository by yourself, let spring boot do that job

}

public Product outOfStock(Long id) {
return productRepository.updateStock(id, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

see comment about updateStock in repository class

@@ -0,0 +1,2 @@
spring.application.name=Backend
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer yml format.

I recommend to rename the file from application.properties to application.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants