Nothing to do if false one-time if

Nothing to do if false one-time if

R=arv
BUG=

Committed: https://github.com/Polymer/TemplateBinding/commit/bec956e

[rafaelw, 2014-01-09 00:34:53]

Committed patchset #1 manually as rbec956e (presubmit successful).

[John Messerly, 2014-01-09 03:14:03]

lgtm :)

[arv, 2014-01-09 15:17:41]

https://codereview.appspot.com/49370043/diff/1/src/TemplateBinding.js
File src/TemplateBinding.js (right):

https://codereview.appspot.com/49370043/diff/1/src/TemplateBinding.js#newcode810
src/TemplateBinding.js:810: return this.updateIteratedValue();
Shouldn't this be?

if (...) {
  this.updateIteratedValue();
  return;
}

since this.updateIteratedValue() does not return void.

[John Messerly, 2014-01-09 20:53:15]

https://codereview.appspot.com/49370043/diff/1/src/TemplateBinding.js
File src/TemplateBinding.js (right):

https://codereview.appspot.com/49370043/diff/1/src/TemplateBinding.js#newcode810
src/TemplateBinding.js:810: return this.updateIteratedValue();
On 2014/01/09 15:17:41, arv wrote:
> Shouldn't this be?
> 
> if (...) {
>   this.updateIteratedValue();
>   return;
> }
> 
> since this.updateIteratedValue() does not return void.

yeah, updateIteratedValue seems a bit inconsistent too, line 838 has `return
this.valueChanged();` but line 849 just has `this.valueChanged(value,
observe);`. Ultimately valueChanged doesn't return anything, so my guess was
that it is shorthand to keep everything on a single line.

[rafaelw1, 2014-01-14 20:00:59]

John's right: I was just trying to be compact. Returns on their own lines
now.


On Thu, Jan 9, 2014 at 12:53 PM, <jmesserly@google.com> wrote:

>
> https://codereview.appspot.com/49370043/diff/1/src/TemplateBinding.js
> File src/TemplateBinding.js (right):
>
> https://codereview.appspot.com/49370043/diff/1/src/
> TemplateBinding.js#newcode810
> src/TemplateBinding.js:810: return this.updateIteratedValue();
> On 2014/01/09 15:17:41, arv wrote:
>
>> Shouldn't this be?
>>
>
>  if (...) {
>>    this.updateIteratedValue();
>>    return;
>> }
>>
>
>  since this.updateIteratedValue() does not return void.
>>
>
> yeah, updateIteratedValue seems a bit inconsistent too, line 838 has
> `return this.valueChanged();` but line 849 just has
> `this.valueChanged(value, observe);`. Ultimately valueChanged doesn't
> return anything, so my guess was that it is shorthand to keep everything
> on a single line.
>
> https://codereview.appspot.com/49370043/
>