Skip to content

Conversation

@gregh3269
Copy link
Contributor

See WW-4607


attr.add("type", "text")
Object type = params.get("type");
attr.add("type", type == null ? "text" : (String) type)
Copy link
Member

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 use a safe way, ie. String.valueOf(type)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did use originally .valueOf and then checking the other handlers they all used the cast, so matched what they use, for code style.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could refactor the rest as well and use String.valueOf or leave it as is, up to you :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Object type = params.get("type");
attr.add("type", type == null ? "text" : type.toString())

valueOf checks for nulls and calls .toString can skip the check?

Copy link
Member

Choose a reason for hiding this comment

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

You must check for null because String.valueOf(null) will return a String "null" :\ I think the current solution is ok :)

@asfgit asfgit merged commit d397abc into apache:master Mar 3, 2016
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.

3 participants