solution#259
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because one core requirement from the task description is not met: the cache must be declared as Map<String, UserData> cache = new ConcurrentHashMap<>(); and store UserData objects, but your current implementation stores CompletableFuture<UserData> instead. This is visible in both AsyncRequestProcessor.java and Main.java, where the cache is typed as ConcurrentHashMap<String, CompletableFuture<UserData>>, which deviates from the specified contract and the expected usage pattern. To fix this, change the field to private final Map<String, UserData> cache = new ConcurrentHashMap<>(); and update processRequest(String userId) so that it first checks cache.get(userId) and, if present, returns CompletableFuture.completedFuture(cachedUserData). If the user is not in the cache, run the asynchronous processing with supplyAsync, create a UserData, store that UserData in the map, and then return the future. Also, please ensure UserData.toString() matches the expected format UserData[userId=user1, details=Details for user1] so the console output is exactly as required. Aside from this cache-type mismatch, your use of CompletableFuture, asynchronous processing, and demonstration in Main are all on the right track, so with these adjustments your solution will fully satisfy the task requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
|
|
||
| public class AsyncRequestProcessor { | ||
| private final Executor executor; | ||
| private ConcurrentHashMap<String, CompletableFuture<UserData>> cache = new ConcurrentHashMap(); |
There was a problem hiding this comment.
The requirements specify Map<String, UserData> cache = new ConcurrentHashMap<>(); and that the cache should store UserData objects, not CompletableFuture<UserData>; consider changing this field to Map<String, UserData> and caching the UserData results instead.
|
|
||
| public CompletableFuture<UserData> processRequest(String userId) { | ||
| return null; | ||
| return this.cache.computeIfAbsent(userId, key -> CompletableFuture.supplyAsync(() -> { |
There was a problem hiding this comment.
Storing the CompletableFuture in the cache deviates from the requirement that the cache holds processed UserData; you can instead check the Map<String, UserData> for an existing entry and, if present, return CompletableFuture.completedFuture(cachedUserData), otherwise process, store the UserData, and return the future.
|
|
||
| public class Main { | ||
| public static void main(String[] args) { | ||
| // Feel free to play with AsyncRequestProcessor in this main method if you want |
There was a problem hiding this comment.
The task description explicitly requires Map<String, UserData> cache = new ConcurrentHashMap<>();; here the field is a ConcurrentHashMap<String, CompletableFuture<UserData>>, which both uses the implementation type instead of the Map interface and stores CompletableFuture rather than UserData, so it doesn’t match the specified cache contract.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because all core functional and technical requirements are implemented correctly: AsyncRequestProcessor uses CompletableFuture with an Executor, the cache is a Map<String, UserData> backed by ConcurrentHashMap, and processRequest correctly checks the cache first and returns a completed future when data is present. Your processRequest(String userId) method simulates delayed computation, creates a UserData with details derived from the userId, stores it in the cache, and returns a CompletableFuture<UserData>, which matches the assignment’s expectations. In Main, you use the specified userIds array ({"user1", "user2", "user3", "user1"}), process them concurrently, and print the results, which satisfies the concurrency and demonstration requirements.
The only thing I recommend double-checking (though it’s not a blocker here) is that UserData.toString() returns exactly the required format, e.g. UserData[userId=user1, details=Details for user1], so that your console output matches the specification character-for-character. Aside from that, your caching logic, asynchronous flow, and overall structure look solid and aligned with the task. Nice work bringing everything together cleanly and fixing the earlier cache-related issues.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.