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

change validate connection func #11

Merged
merged 2 commits into from
Aug 16, 2021
Merged

Conversation

Idokah
Copy link
Collaborator

@Idokah Idokah commented Aug 12, 2021

validateConnection function returns object {valid: true/false, error} instead throw
adjust the tests.

@Idokah Idokah force-pushed the change-validate-connection branch from c284717 to 9f0adeb Compare August 15, 2021 08:01
@Idokah Idokah requested review from noam-almog and MXPOL August 15, 2021 10:59
res = { valid: false, error }
}
}
return res;
Copy link
Collaborator

Choose a reason for hiding this comment

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

i don't understand this change, looks like debug code to me

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you suggested me to change the validateConnection function to return an object with valid = true/false and the error instead of throwing if the connection is not valid.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, so why do we need to extract all of the logic to try catch ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because i want to use the translateErrorCodes function, this function throws the error so i need to catch it..
I know it's ugly, you have better idea to return the error without throwing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

so create another function like translateErrorCodes that does that without throwing

@Idokah Idokah force-pushed the change-validate-connection branch 2 times, most recently from bad4592 to 10a21b6 Compare August 16, 2021 08:21
@Idokah Idokah force-pushed the change-validate-connection branch from 10a21b6 to 8eeacf2 Compare August 16, 2021 11:47
@Idokah Idokah force-pushed the change-validate-connection branch from 8eeacf2 to 1050b89 Compare August 16, 2021 12:06
@Idokah Idokah merged commit c9ae868 into master Aug 16, 2021
@noam-almog noam-almog deleted the change-validate-connection branch August 26, 2021 14:48
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.

2 participants