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

Authentification service branch #1067

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

Conversation

xli1iax
Copy link

@xli1iax xli1iax commented Jul 20, 2024

No description provided.

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

Choose a reason for hiding this comment

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

Suggested change
UserService user = new UserService();
UserService userService = new UserService();

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

Choose a reason for hiding this comment

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

create object outside of method

@@ -11,6 +13,13 @@ public class AuthenticationService {
* Return false in any other cases.
*/
public boolean login(String email, String password) {
UserService user = new UserService();
User found = user.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 found = user.findByEmail(email);
User user = userService.findByEmail(email);

Comment on lines 18 to 23
if (found != null) {
if (found.getPassword().equals(password)) {
return true;
}
}
return false;

Choose a reason for hiding this comment

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

simplify it using one return

@@ -15,6 +15,11 @@ public class UserService {
* Return <code>null</code> if there is no suitable user
*/
public User findByEmail(String email) {
for (User i: users) {

Choose a reason for hiding this comment

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

Suggested change
for (User i: users) {
for (User user : users) {

Choose a reason for hiding this comment

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

not fixed, rename variable

@@ -11,6 +13,9 @@ public class AuthenticationService {
* Return false in any other cases.
*/
public boolean login(String email, String password) {
return false;
UserService userService = new UserService();
User userToCheck = 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 userToCheck = userService.findByEmail(email);
User user = userService.findByEmail(email);

@@ -15,6 +15,11 @@ public class UserService {
* Return <code>null</code> if there is no suitable user
*/
public User findByEmail(String email) {
for (User i: users) {

Choose a reason for hiding this comment

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

not fixed, rename variable

@xli1iax xli1iax requested a review from Elena-Bruyako July 21, 2024 19:18
@@ -10,7 +12,15 @@ public class AuthenticationService {
* @return true if user by email exists and passed password is equal to user's password.
* Return false in any other cases.
*/
private UserService userService = new UserService();

Choose a reason for hiding this comment

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

Suggested change
private UserService userService = new UserService();
private final UserService userService = new UserService();

Comment on lines 17 to 19
public UserService getUserService() {
return userService;
}

Choose a reason for hiding this comment

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

Suggested change
public UserService getUserService() {
return userService;
}

public boolean login(String email, String password) {
return false;
User userToCheck = getUserService().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 userToCheck = getUserService().findByEmail(email);
User user =userService.findByEmail(email);

@xli1iax xli1iax requested a review from Elena-Bruyako July 21, 2024 20:53
@@ -10,7 +12,17 @@ public class AuthenticationService {
* @return true if user by email exists and passed password is equal to user's password.
* Return false in any other cases.
*/
private final UserService userService = new UserService();

public UserService getUserService() {

Choose a reason for hiding this comment

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

Why do you need this get method?

Copy link
Author

@xli1iax xli1iax Jul 22, 2024

Choose a reason for hiding this comment

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

to get the variable, Intellij IDEA indicated that it is better to make UserService object private and to get it I created the get method

Choose a reason for hiding this comment

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

Not all advice from Idea should be implemented

Choose a reason for hiding this comment

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

remove this get method

@xli1iax xli1iax requested a review from Elena-Bruyako July 22, 2024 16:01
@@ -10,7 +12,17 @@ public class AuthenticationService {
* @return true if user by email exists and passed password is equal to user's password.
* Return false in any other cases.
*/
private final UserService userService = new UserService();

public UserService getUserService() {

Choose a reason for hiding this comment

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

remove this get method

public boolean login(String email, String password) {
return false;
User user = getUserService().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 user = getUserService().findByEmail(email);
User user = userService.findByEmail(email);

@@ -10,7 +12,17 @@ public class AuthenticationService {
* @return true if user by email exists and passed password is equal to user's password.
* Return false in any other cases.
*/
private final UserService userService = new UserService();

Choose a reason for hiding this comment

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

Your code should look like

Suggested change
private final UserService userService = new UserService();
private final UserService userService = new UserService();
public boolean login(String email, String password) {
User user = userService.findByEmail(email);
return user != null && user.getPassword().equals(password);
}

@xli1iax xli1iax requested a review from Elena-Bruyako July 22, 2024 16:56
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.

3 participants