[Pushover] Default Value Usage Fix #5123
Conversation
|
Helping support the use-case in this thread https://community.openhab.org/t/pushover-action-syntax/23188 , without changing or breaking previous functional uses. |
9037568
left a comment
There was a problem hiding this comment.
Aside from the two inline comments, this LGTM.
However, calling a method with a dozen parameters when most of them are null seems like a poor design that would benefit from a rethink.
| if (defaultUrl.length() <= API_MAX_URL_LENGTH) { | ||
| addEncodedParameter(data, MESSAGE_KEY_URL, defaultUrl); | ||
| } else { | ||
| logger.error("The url is greater than " + API_MAX_URL_LENGTH + " characters."); |
There was a problem hiding this comment.
Should use parameterized logging instead of string concatenation.
| if (defaultUrlTitle.length() <= API_MAX_URL_TITLE_LENGTH) { | ||
| addEncodedParameter(data, MESSAGE_KEY_URL_TITLE, defaultUrlTitle); | ||
| } else { | ||
| logger.error("The url title is greater than " + API_MAX_URL_TITLE_LENGTH + " characters."); |
There was a problem hiding this comment.
Should use parameterized logging instead of string concatenation.
|
|
||
| if (!StringUtils.isEmpty(apiKey)) { | ||
| addEncodedParameter(data, MESSAGE_KEY_API_KEY, apiKey); | ||
| } else if (StringUtils.isEmpty(apiKey) && !StringUtils.isEmpty(defaultApiKey)) { |
There was a problem hiding this comment.
The part StringUtils.isEmpty(apiKey) && is unneeded and would read better if it were removed.
|
|
||
| if (!StringUtils.isEmpty(user)) { | ||
| addEncodedParameter(data, MESSAGE_KEY_USER, user); | ||
| } else if (StringUtils.isEmpty(user) && !StringUtils.isEmpty(defaultUser)) { |
There was a problem hiding this comment.
The part StringUtils.isEmpty(user) && is unneeded and would read better if it were removed.
|
|
||
| if (!StringUtils.isEmpty(device)) { | ||
| addEncodedParameter(data, MESSAGE_KEY_DEVICE, device); | ||
| } else if (StringUtils.isEmpty(device) && !StringUtils.isEmpty(defaultDevice)) { |
There was a problem hiding this comment.
The part StringUtils.isEmpty(device) && is unneeded and would read better if it were removed.
|
|
||
| if (!StringUtils.isEmpty(title)) { | ||
| addEncodedParameter(data, MESSAGE_KEY_TITLE, title); | ||
| } else if (StringUtils.isEmpty(title) && !StringUtils.isEmpty(defaultTitle)) { |
There was a problem hiding this comment.
The part StringUtils.isEmpty(title) && is unneeded and would read better if it were removed.
| logger.error("The url is greater than " + API_MAX_URL_LENGTH + " characters."); | ||
| return false; | ||
| } | ||
| } else if (StringUtils.isEmpty(url) && !StringUtils.isEmpty(defaultUrl)) { |
There was a problem hiding this comment.
The part StringUtils.isEmpty(url) && is unneeded and would read better if it were removed.
| logger.error("The url title is greater than " + API_MAX_URL_TITLE_LENGTH + " characters."); | ||
| return false; | ||
| } | ||
| } else if (StringUtils.isEmpty(urlTitle) && !StringUtils.isEmpty(defaultUrlTitle)) { |
There was a problem hiding this comment.
The part StringUtils.isEmpty(urlTitle) && is unneeded and would read better if it were removed.
|
|
||
| if (!StringUtils.isEmpty(sound)) { | ||
| addEncodedParameter(data, MESSAGE_KEY_SOUND, sound); | ||
| } else if (StringUtils.isEmpty(sound) && !StringUtils.isEmpty(defaultSound)) { |
There was a problem hiding this comment.
The part StringUtils.isEmpty(sound) && is unneeded and would read better if it were removed.
76998f7 to
3d78ab1
Compare
when not supplied.
3d78ab1 to
949fd76
Compare
|
Changes to the code based on review as requested. Signed-off-by: Chris Graham crackerstealth@gmail.com (github: CrackerStealth) |
The changes reflect the this PR: #5123
Updated pushover to allow the main pushover call to use default values when not supplied.
Signed-off-by: Chris Graham crackerstealth@gmail.com (github: CrackerStealth)