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

added requested methods #1164

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

kostya-savchenko
Copy link

No description provided.

@@ -9,6 +9,10 @@ public User(String email, String password) {
this.password = password;
}

public User(String email) {

Choose a reason for hiding this comment

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

remove constructor

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@@ -15,6 +15,10 @@ public class UserService {
* Return <code>null</code> if there is no suitable user
*/
public User findByEmail(String email) {
return null;
if (email == "[email protected]" || email == "[email protected]") {

Choose a reason for hiding this comment

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

in for loop get from users array user.getEmail() and compare it with method argument email

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@@ -11,6 +11,11 @@ public class AuthenticationService {
* Return false in any other cases.
*/
public boolean login(String email, String password) {
return false;
if (email.equals("[email protected]") && password.equals("1234") || email.equals("[email protected]")

Choose a reason for hiding this comment

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

Create instanse of UserService
find user by email
return true if user by email exists and passed password is equal to user's password

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@@ -11,6 +13,12 @@ public class AuthenticationService {
* Return false in any other cases.
*/
public boolean login(String email, String password) {
return false;
UserService userService = new UserService();

Choose a reason for hiding this comment

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

it should be class-level variable, don't need to create new UserService() each time when login method is called

Copy link
Author

Choose a reason for hiding this comment

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

corrected

Comment on lines 18 to 22
if (foundUser != null && password.equals(foundUser.getPassword())) {
return true;
} else {
return false;
}

Choose a reason for hiding this comment

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

Suggested change
if (foundUser != null && password.equals(foundUser.getPassword())) {
return true;
} else {
return false;
}
return foundUser != null && password.equals(foundUser.getPassword());

Copy link
Author

Choose a reason for hiding this comment

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

added suggestion

Copy link

@sarakhmen sarakhmen left a comment

Choose a reason for hiding this comment

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

let's slightly improve your solution :)

@@ -1,16 +1,20 @@
package mate.academy.service;

public class AuthenticationService {
private UserService userService = new UserService();
// User foundUser = userService.findByEmail(email);

Choose a reason for hiding this comment

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

Suggested change
// User foundUser = userService.findByEmail(email);

remove redundant commented lines

* @return true if user by email exists and passed password is equal to user's password.
* Return false in any other cases.
*/

Choose a reason for hiding this comment

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

Suggested change

@@ -3,6 +3,7 @@
import mate.academy.model.User;

public class UserService {

Choose a reason for hiding this comment

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

Suggested change

Copy link

@liliia-ponomarenko liliia-ponomarenko left a comment

Choose a reason for hiding this comment

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

Good job! One recommendation ;)

Comment on lines +16 to +17
return (userService.findByEmail(email) != null && password
.equals(userService.findByEmail(email).getPassword())) ? true : false;

Choose a reason for hiding this comment

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

better create separate variable for userService.findByEmail(email) and avoid redundant call of this method

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.

4 participants