feat: implement LogEntry model and pass unit tests#8
Open
ANKUSHG11 wants to merge 1 commit intoignorant05:mainfrom
Open
feat: implement LogEntry model and pass unit tests#8ANKUSHG11 wants to merge 1 commit intoignorant05:mainfrom
ANKUSHG11 wants to merge 1 commit intoignorant05:mainfrom
Conversation
ignorant05
reviewed
Feb 7, 2026
Owner
ignorant05
left a comment
ignorant05
left a comment
There was a problem hiding this comment.
LGTM, but it is still doesn't conform with the intended use, you need just some little tweaks and it'll pas.
And the best practice is to separate to use two commit:
- One for
LogEntryclass and it's unit tests - One for
JsonUtilclass and it's unit tests.
so that it becomes more organized.
Thanks for your work, much appreciated.
| private String message; | ||
| private String timestamp; | ||
|
|
||
| public LogEntry(String level, String message) { |
Owner
There was a problem hiding this comment.
So far, it is good but i'd like to add a bit more attributes and modify some:
private String id; // this is good
private Instant timestamp; // i'd like it to be of type Instant, so don't toString it
private String level; // this is good
private String service; // i like the logs to be more specific, i.g; adding the exact source/service of the log in microservices architecture
private String message; // this is good
private String userID; // i'd like it to be more user specific too
private String ipAddr; // the ip address is needed for potential network troubleshooting need or geographical specific log tracking
private int durationMS; // this is needed for performance monitoring, to know how long the logging operation took| this.timestamp = Instant.now().toString(); | ||
| } | ||
|
|
||
| // Getters |
Owner
There was a problem hiding this comment.
Update getters to match the intended attributes and their type.
| this.message = message; | ||
| this.timestamp = Instant.now().toString(); | ||
| } | ||
|
|
Owner
There was a problem hiding this comment.
For potential need, can you please add setters too ?
| public String getTimestamp() { return timestamp; } | ||
|
|
||
| // Serialization for the task | ||
| public String toJson() { |
Owner
There was a problem hiding this comment.
this is good for a tarting point, but i wish that you can migrate it to ./src/main/java/com/github/ignorant05/log_processing_system/util/JsonUtil.java and add it's own unit tests in the ./src/test/java/com/github/ignorant05/log_processing_system/util/JsonUtilTest.java file
Note: if you run into a problem with this and didn't understand how will it work, you can either ask me and i'll help you out.
| "{\"id\":\"%s\", \"level\":\"%s\", \"message\":\"%s\", \"timestamp\":\"%s\"}", | ||
| id, level, message, timestamp | ||
| ); | ||
| } |
Owner
There was a problem hiding this comment.
i'd like to add more methods to the main LogEntry class, since it is merely hardcoded data, i wish that you can:
- Override the
equalsmethod which takes an object of type Object (import this : java.util.Objects) and compare each attribute with the current instance, - Override
hashcodemethod, it returns a hashed version of the current set of attribute values, use Objects.hash method for this. - Override
toStringmethod, to return it in a more custom format, i predfer if you separate each attribute/value combination with a new line and add an indentation at each one's begginng. - Add more unit tests to test the intended results with each one, i want these kind of unit tests,
shouldCreateLogEntryWithConstructor(),shouldSetAndGetFields(),shouldImplementEqualsAndHashCode(),shouldProduceValidStringRepresentation()andshouldCreateLogEntryWithBuilder(), you can add more if you want, but do tell me why about it's utility and why you see it is needed.
| private String level; | ||
| private String message; | ||
| private String timestamp; | ||
|
|
Owner
There was a problem hiding this comment.
add a default empty constructor here please.
| public String getLevel() { return level; } | ||
| public String getMessage() { return message; } | ||
| public String getTimestamp() { return timestamp; } | ||
|
|
Owner
There was a problem hiding this comment.
add a builder class to avoid the pain that comes with setters :
here's a snippet of what i want it to be like, and don't forget the unit tests please, even a single one for this suffices, but this is much needed.
public static Builder builder() {
return new Builder();
}
public static class Builder {
private String id;
private Instant timestamp;
private String level;
private String service;
private String message;
private String userID;
private String ipAddr;
private int durationMS;
public Builder id(String id) {
this.id = id;
return this;
}
public Builder timestamp(Instant timestamp) {
this.timestamp = timestamp;
return this;
}
public Builder level(String level) {
this.level = level;
return this;
}
public Builder service(String service) {
this.service = service;
return this;
}
public Builder message(String message) {
this.message = message;
return this;
}
public Builder userID(String userID) {
this.userID = userID;
return this;
}
public Builder ipAddr(String ipAddr) {
this.ipAddr = ipAddr;
return this;
}
public Builder durationMS(int durationMS) {
this.durationMS = durationMS;
return this;
}
public LogEntry build() {
return new LogEntry(id, timestamp, level, service, message, userID, ipAddr, durationMS);
}
}
Author
|
Hi @ignorant05, thank you for the detailed feedback! Since I am currently
in the early stages of learning Core Java, these advanced changes (like the
Builder pattern and utility separation) are a bit overwhelming for me right
now.
I have done my best with the initial implementation. I will continue my
learning journey and hope to contribute more effectively to your project
once I have a better grasp of these concepts. Thanks for the opportunity!"
…On Sat, 7 Feb, 2026, 3:15 pm pebble, ***@***.***> wrote:
***@***.**** commented on this pull request.
LGTM, but it is still doesn't conform with the intended use, you need just
some little tweaks and it'll pas.
And the best practice is to separate to use two commit:
- One for LogEntry class and it's unit tests
- One for JsonUtil class and it's unit tests.
so that it becomes more organized.
Thanks for your work, much appreciated.
------------------------------
In src/main/java/com/github/ignorant05/log_processing_system/LogEntry.java
<#8 (comment)>
:
> @@ -0,0 +1,32 @@
+package com.github.ignorant05.log_processing_system;
+
+import java.time.Instant;
+import java.util.UUID;
+
+public class LogEntry {
+ private String id;
+ private String level;
+ private String message;
+ private String timestamp;
+
+ public LogEntry(String level, String message) {
So far, it is good but i'd like to add a bit more attributes and modify
some:
private String id; (this is good)
private Instant timestamp; (i'd like it to be of type Instant, so don't
toString it)
private String level; (this is good)
private String service; (i like the logs to be more specific, i.g; adding
the exact source/service of the log in microservices architecture)
private String message;
private String userID; (i'd like it to be more user specific too)
private String ipAddr; (the ip address is needed for potential network
troubleshooting need or geographical specific log tracking)
private int durationMS; (this is needed for performance monitoring, to
know how long the logging operation took)
------------------------------
In src/main/java/com/github/ignorant05/log_processing_system/LogEntry.java
<#8 (comment)>
:
> +import java.util.UUID;
+
+public class LogEntry {
+ private String id;
+ private String level;
+ private String message;
+ private String timestamp;
+
+ public LogEntry(String level, String message) {
+ this.id = UUID.randomUUID().toString();
+ this.level = level;
+ this.message = message;
+ this.timestamp = Instant.now().toString();
+ }
+
+ // Getters
Update getters to match the intended attributes and their type.
------------------------------
In src/main/java/com/github/ignorant05/log_processing_system/LogEntry.java
<#8 (comment)>
:
> +import java.time.Instant;
+import java.util.UUID;
+
+public class LogEntry {
+ private String id;
+ private String level;
+ private String message;
+ private String timestamp;
+
+ public LogEntry(String level, String message) {
+ this.id = UUID.randomUUID().toString();
+ this.level = level;
+ this.message = message;
+ this.timestamp = Instant.now().toString();
+ }
+
For potential need, can you please add setters too ?
------------------------------
In src/main/java/com/github/ignorant05/log_processing_system/LogEntry.java
<#8 (comment)>
:
> +
+ public LogEntry(String level, String message) {
+ this.id = UUID.randomUUID().toString();
+ this.level = level;
+ this.message = message;
+ this.timestamp = Instant.now().toString();
+ }
+
+ // Getters
+ public String getId() { return id; }
+ public String getLevel() { return level; }
+ public String getMessage() { return message; }
+ public String getTimestamp() { return timestamp; }
+
+ // Serialization for the task
+ public String toJson() {
this is good for a tarting point, but i wish that you can migrate it to
./src/main/java/com/github/ignorant05/log_processing_system/util/JsonUtil.java
and add it's own unit tests in the
./src/test/java/com/github/ignorant05/log_processing_system/util/JsonUtilTest.java
file
------------------------------
In src/main/java/com/github/ignorant05/log_processing_system/LogEntry.java
<#8 (comment)>
:
> + this.timestamp = Instant.now().toString();
+ }
+
+ // Getters
+ public String getId() { return id; }
+ public String getLevel() { return level; }
+ public String getMessage() { return message; }
+ public String getTimestamp() { return timestamp; }
+
+ // Serialization for the task
+ public String toJson() {
+ return String.format(
+ "{\"id\":\"%s\", \"level\":\"%s\", \"message\":\"%s\", \"timestamp\":\"%s\"}",
+ id, level, message, timestamp
+ );
+ }
i'd like to add more methods to the main LogEntry class, since it is
merely hardcoded data, i wish that you can:
- Override the equals method which takes an object of type Object
(import this : *java.util.Objects*) and compare each attribute with
the current instance,
- Override hashcode method, it returns a hashed version of the current
set of attribute values, use *Objects.hash* method for this.
- Override toString method, to return it in a more custom format, i
predfer if you separate each attribute/value combination with a new line
and add an indentation at each one's begginng.
- Add more unit tests to test the intended results with each one, i
want these kind of unit tests, shouldCreateLogEntryWithConstructor(), shouldSetAndGetFields(),
shouldImplementEqualsAndHashCode(),
shouldProduceValidStringRepresentation()and shouldCreateLogEntryWithBuilder()`,
you can add more if you want, but do tell me why about it's utility and why
you see it is needed.
------------------------------
In src/main/java/com/github/ignorant05/log_processing_system/LogEntry.java
<#8 (comment)>
:
> @@ -0,0 +1,32 @@
+package com.github.ignorant05.log_processing_system;
+
+import java.time.Instant;
+import java.util.UUID;
+
+public class LogEntry {
+ private String id;
+ private String level;
+ private String message;
+ private String timestamp;
+
add a default empty constructor here please.
------------------------------
In src/main/java/com/github/ignorant05/log_processing_system/LogEntry.java
<#8 (comment)>
:
> + private String message;
+ private String timestamp;
+
+ public LogEntry(String level, String message) {
+ this.id = UUID.randomUUID().toString();
+ this.level = level;
+ this.message = message;
+ this.timestamp = Instant.now().toString();
+ }
+
+ // Getters
+ public String getId() { return id; }
+ public String getLevel() { return level; }
+ public String getMessage() { return message; }
+ public String getTimestamp() { return timestamp; }
+
add a builder class to avoid the pain that comes with setters :
here's a snippet of what i want it to be like, and don't forget the unit
tests please, even a single one for this suffices, but this is much needed.
public static Builder builder() {
return new Builder();
}
public static class Builder {
private String id;
private Instant timestamp;
private String level;
private String service;
private String message;
private String userID;
private String ipAddr;
private int durationMS;
public Builder id(String id) {
this.id = id;
return this;
}
public Builder timestamp(Instant timestamp) {
this.timestamp = timestamp;
return this;
}
public Builder level(String level) {
this.level = level;
return this;
}
public Builder service(String service) {
this.service = service;
return this;
}
public Builder message(String message) {
this.message = message;
return this;
}
public Builder userID(String userID) {
this.userID = userID;
return this;
}
public Builder ipAddr(String ipAddr) {
this.ipAddr = ipAddr;
return this;
}
public Builder durationMS(int durationMS) {
this.durationMS = durationMS;
return this;
}
public LogEntry build() {
return new LogEntry(id, timestamp, level, service, message, userID, ipAddr, durationMS);
}
}
—
Reply to this email directly, view it on GitHub
<#8 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BQJFHP33BOS4MIJDQWRIBWD4KWX4LAVCNFSM6AAAAACUJ447ISVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTONRWG43TANZVGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Owner
|
Hi @ANKUSHG11, i appreciate you enthusiasm about contributing to the project, and i am not demanding many things, for now, you can just try to implement what you can gasp from the feedback i wrote earlier and i'll take it from there, you'll save me sometime by doing that. Again, i much appreciate your contribution, feel free to see future issues and try your best if you want. |
Author
|
Thank you for understanding! For now, I'm not very comfortable with these
changes as I want to focus on my Core Java basics first. I'll leave the PR
as it is, and feel free to close it or modify it. I'll definitely look for
more issues once I've learned more. Thanks again!
…On Sat, 7 Feb, 2026, 3:52 pm pebble, ***@***.***> wrote:
*ignorant05* left a comment (ignorant05/log-processing-system#8)
<#8 (comment)>
Hi @ANKUSHG11 <https://github.com/ANKUSHG11>, i appreciate you enthusiasm
about contributing to the project, and i am not demanding many things, for
now, you can just try to implement what you can gasp from the feedback i
wrote earlier and i'll take it from there, you'll save me sometime by doing
that.
Again, i much appreciate your contribution, feel free to see future issues
and try your best if you want.
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BQJFHP4LIEXQQEM525YLHVD4KW4GTAVCNFSM6AAAAACUJ447ISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQNRUGEYDIMBWHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Owner
|
It's fine, looking forward for your contribution in the future. |
Author
|
Thank you for your understanding.
…On Sat, 7 Feb, 2026, 5:31 pm pebble, ***@***.***> wrote:
*ignorant05* left a comment (ignorant05/log-processing-system#8)
<#8 (comment)>
It's fine, looking forward for your contribution in the future.
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BQJFHP4YCI4MNJJXBAC2V3T4KXHYHAVCNFSM6AAAAACUJ447ISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQNRUGM3DQMJUGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have implemented the LogEntry class and added unit tests as requested in the issue. All tests passed locally.