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

update #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

update #3

wants to merge 2 commits into from

Conversation

garikhgh
Copy link
Owner

@garikhgh garikhgh commented May 2, 2023

No description provided.

@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long productId;
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
Copy link
Collaborator

Choose a reason for hiding this comment

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

better to use sequence

}
@Override
public int hashCode() {
return getClass().hashCode();
Copy link
Collaborator

Choose a reason for hiding this comment

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

not a good idea. equals and hashcode should be interconnected. please use the same fields

@@ -9,26 +9,27 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;


@Service
@RequiredArgsConstructor
@Slf4j
public class ProductFacade implements ProductUniquenessFacade {
Copy link
Collaborator

Choose a reason for hiding this comment

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

better to make package-private to not allow others to use it

@@ -8,22 +8,23 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;


@Service
@RequiredArgsConstructor
@Slf4j
public class ProductService implements ProductDomainService {
Copy link
Collaborator

Choose a reason for hiding this comment

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

better to make package-private

}
public MetricsService(MeterRegistry registry) {
log.info("Registering product countMetric into the Prometheus.");
Gauge.builder("product_counter", fetchProductCount())
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's use Counter instead

@@ -7,23 +7,24 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;


@RequiredArgsConstructor
@Slf4j
@Service
public class ProductCounterService {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

log.info("Checking if the product is unique in the in memory counter {}", productDto.getName());
var productCounter = ProductCounterService.getProductCounter();
@Override
public ResponseMessage countProducts(ProductDto productDto) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

the name is not good. maybe create is better name

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.

2 participants