-
Notifications
You must be signed in to change notification settings - Fork 24
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
@W-14781712 fix: added null checks #143
Conversation
Thanks for the contribution! It looks like @abbasmanthirik is an internal user so signing the CLA is not required. However, we need to confirm this. |
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.
We have a few queries:
Code Change related:
- Why a null check has been added here for W-14781712 where EntityFormulaContextBase.createMockFieldInfo is throwing the "IllegalArgumentException: entity Info is null"? How is this check related to that exception?
Code Quality related: - Why there is a need for the "else" block?
- Why the same check has not been added to the another constructor of the same class?
- Why has the datatype been updated from primitive to object for location and type? If yes, then does the getter/setter need to be updated or not, has that been considered?
`
|
@@ -64,8 +70,13 @@ public WrongArgumentTypeException(String function, Type[] expectedInputTypes, Fo | |||
|
|||
Token token = actual.getToken(); | |||
location = token.getColumn(); |
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.
Why is the "location" outside the null check here?
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.
Was a miss. Fixed it. Thanks
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.
Why is the "location" outside the null check here?
Done
Added null checks for token