-
Notifications
You must be signed in to change notification settings - Fork 936
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
all tasks are solved #1163
base: master
Are you sure you want to change the base?
all tasks are solved #1163
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job! Let’s improve your solution ;)
for (User element: users) { | ||
if (email.equals(element.getEmail())) { | ||
return element; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (User element: users) { | |
if (email.equals(element.getEmail())) { | |
return element; | |
} | |
for (User user: users) { | |
if (email.equals(user.getEmail())) { | |
return user; | |
} |
@@ -11,6 +13,17 @@ public class AuthenticationService { | |||
* Return false in any other cases. | |||
*/ | |||
public boolean login(String email, String password) { | |||
return false; | |||
UserService newUserSerice = new UserService(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better make it class level variable
User user1 = newUserSerice.findByEmail(email); | ||
if (user1 == null) { | ||
return false; | ||
} else { | ||
String correctPassword = user1.getPassword(); | ||
if (password.equals(correctPassword)) { | ||
return true; | ||
} else { | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid nesting conditional operators. One if
block is enough, think about how to combine conditions and simplify your solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job, you can improve your solution if you want ;)
User user1 = newUserSerice.findByEmail(email); | ||
return user1 != null && password.equals(user1.getPassword()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User user1 = newUserSerice.findByEmail(email); | |
return user1 != null && password.equals(user1.getPassword()); | |
User user = newUserSerice.findByEmail(email); | |
return user != null && password.equals(user.getPassword()); |
actually it's a bad practice to add such suffixes to the var names
I implemented a login method in AuthenticationService and findByEmail method in UserService