Skip to content
This repository was archived by the owner on Jul 7, 2020. It is now read-only.

Conversation

@georgekankava
Copy link

This pull request is focused on resolving occurrences of Sonar rule
squid:S2259 - Null pointers should not be dereferenced.
You can find more information about the issue here:
https://dev.eclipse.org/sonar/coding_rules#q=squid%3AS2259
Please let me know if you have any questions.
George Kankava

log.warn("Class '" + clazz + "' registered for '" + macroName + "' cannot be initialized: " + e, e);
} catch (ClassCastException e) {
log.warn("Class '" + clazz + "' registered for '" + macroName + "' is not JobConfigExpander but '" + o.getClass() + "'");
log.warn("Class '" + clazz + "' registered for '" + macroName + "' is not JobConfigExpander but '" + getObjectClassIfNotNull(o) + "'");
Copy link
Contributor

Choose a reason for hiding this comment

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

o is guaranteed to be not null, so the extra guard is unnecessary here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be better to split the try block into two. A bit more verbose, but it would be clearer which catch blocks are guarding which line. Something something clarity is great.

It's been an awfully long time, so I suggest closing this PR (with gratitude), and then fixing whatever you want from a cleaner commit.

@Override
public boolean equals(Object o) {
if (o.getClass() != getClass()) {
if (o == null || o.getClass() != getClass()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A fun alternative here is a humble instanceof call. It'll return false for both nulls and guys two lines away from a class cast exception.

One time I checked the jvm bytecode specifically to spite an old professor and at the time at least, it was also several instructions more efficient.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants