Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
create EndorsementController #41
create EndorsementController #41
Changes from 13 commits
0fdea86
9a89d07
0d0aa10
3175cc6
c9af284
e90b5f2
d1faad5
c0e211e
39cf6f9
1f6c2d4
ca1b171
0fde179
a8a2e5e
e867d0f
be11ae5
2b17be2
3cacabe
2e8bbe0
2084c09
d89effd
ba72dd8
08bf445
77f7110
5742947
c5a09ca
f4cbbbd
05237e6
717f24d
eae68d5
9ed6eb3
f3f9894
bb7ae49
2fab76d
3acd3d5
d90443f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is there a reason why this is a
RuntimeException
and notException
?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.
An Exception is checked, and a RuntimeException is unchecked.
Checked means that the compiler requires that you handle the exception in a catch, or declare your method as throwing it (or one of its superclasses).
Generally, throw a checked exception if the caller of the API is expected to handle the exception, and an unchecked exception if it is something the caller would not normally be able to handle, such as an error with one of the parameters, i.e. a programming mistake.
So here we are using these error to throw error for things the caller API couldn't have checked before hand like, "no user foiund for this user id " etc
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.
@bhtibrewal and @debanjanc01 what about overwriting the stack trace in case of check and un-checked (question for both)
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.
@iamitprakash if you're talking about custom exceptions, then in my opinion it depends on how the exception is being used.
If it's Exceptions that ultimately propagates directly to the Controller and is handled by a global exception handler, then the stack trace can be suppressed. The assumption here is that the exception handler will be sending the correct response to the API caller.
If it's Exceptions that are being used internally, and handled via Catch statements, we might want to error log with the stacktrace.
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.
I don't the knowledge on this topic, need to read more
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.
Isn't
boolean
false by default?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.
yes thanks for pointing out wasn't aware, removed it
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.
Any use for this class?
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.
This is there in different PR, we can remove this @bhtibrewal