Type Assertion Paranoia #121
Conversation
A helper function helps ensure this is handled more consistently, and more safely. Refs: - https://issues.redhat.com/browse/ACM-4309 Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
This real type removes the need for type assertions in the `createInformStatus` function. Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
JustinKuli
force-pushed
the
4309-null-panics
branch
from
d5fb50f
to
e8dd0db
Compare
This may help performance slightly, and removes the need for some type assertions in the `mergeArrays` function. Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
JustinKuli
force-pushed
the
4309-null-panics
branch
from
e8dd0db
to
e58df3c
Compare
JustinKuli
force-pushed
the
4309-null-panics
branch
from
e58df3c
to
2957adb
Compare
| if !checkFieldsWithSort(mVal, oldItem[i].(map[string]interface{})) { | ||
| return false | ||
| if oVal, ok := oldItem[i].(map[string]interface{}); ok { | ||
| return len(mVal) == len(oVal) && checkFieldsWithSort(mVal, oVal) |
There was a problem hiding this comment.
Would it need to return false after this since ok would have failed, similar to above?
There was a problem hiding this comment.
Probably. Good catch.
I started to get distracted making all of these kinds of things more consistent... but I think that it needs more attention than I can give it right now. It is probably due for some refactoring to reduce some repeated logic like length checks before and in checkListsMatch. So, maybe that's a future item.
Some of these may be bordering on paranoia, and my search was not totally exhaustive... but this should make me feel a little bit safer. Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
JustinKuli
force-pushed
the
4309-null-panics
branch
from
2957adb
to
f142739
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhaiducek, JustinKuli The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
While working on https://issues.redhat.com/browse/ACM-4309, I found a few more spots where it's hard to tell if type assertions could panic... well, now it's easier, because in these instances, it definitely can't anymore.