Skip to content

Salem B#6

Open
Barboud wants to merge 2 commits into
HackYourAssignment:mainfrom
Barboud:main
Open

Salem B#6
Barboud wants to merge 2 commits into
HackYourAssignment:mainfrom
Barboud:main

Conversation

@Barboud

@Barboud Barboud commented May 30, 2026

Copy link
Copy Markdown

No description provided.

public AnalyticsRecord create(CreateAnalyticsRequest request) {
AnalyticsRecord record = new AnalyticsRecord();

record.setTraceId(String.valueOf(counter));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is not thread-safe. Multiple simultaneous requests can result in records with the same traceId. It would also be better to used UUID for the unique ids instead of just numbers.

import static org.junit.jupiter.api.Assertions.*;

public class AnalyticsServiceTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This only tests the create method. A lot of methods are still untested. Test coverage should be improved.

@@ -0,0 +1,8 @@
package com.hyf.AnalyticsAPI.exception;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For this exception to work an error handler is still needed. No I get 500 errors instead of 404 when items are not found.

## Delete Record

DELETE /analytics/{id}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider returning 204 here or return a body with some additional information.

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