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

Issue-Fix-#819 append: toString methods #823

Merged
merged 3 commits into from Jan 15, 2024

Conversation

l3002
Copy link
Contributor

@l3002 l3002 commented Jan 13, 2024

Fixes #819

Description

Overrode toString() method for following classes:

  • User - prints ID and Email (wasn't able to find the username for this class therefore used email instead)
  • Game - prints ID and Title

Signed-off-by: l3002 <tanmaymathpal4545@gmail.com>
Copy link
Member

@Brutus5000 Brutus5000 left a comment

Choose a reason for hiding this comment

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

Please just use the variable names as is. I know I wrote title in the issue description, but I wasn't aware of the variables name.

@@ -19,4 +19,10 @@ public class User extends Login {
public String getPassword() {
return password;
}

//Overriding toString method for efficient Logging
Copy link
Member

Choose a reason for hiding this comment

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

Is there a particular reason not to use Lombok here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, If we use the ToString Annotation for User class then that annotation will have to call the super classes toString methods recursively as the User class inherit those properties from the classes up in the heirarchy, which leads to a messy format. Something like this User(super(super(id = 1232)), super(email = xyz@gmail.com)).

This is the reason why I used the traditional way of overriding the toString() method

Copy link
Member

Choose a reason for hiding this comment

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

ok. But please make the fields names lowercase :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Brutus5000 I've added two new commits to this PR. Could you please check if the naming convention is alright now?

Signed-off-by: l3002 <tanmaymathpal4545@gmail.com>
Signed-off-by: l3002 <tanmaymathpal4545@gmail.com>
@Brutus5000 Brutus5000 merged commit b9bbb95 into FAForever:develop Jan 15, 2024
1 check passed
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.

Add toString() methods for regularly logged entities
2 participants