crysalead-js / dom-layer

Virtual DOM implementation.
MIT License
30 stars 1 forks source link

Confusing and bad code #25

Closed ghost closed 9 years ago

ghost commented 9 years ago

Hi. I just want to let you know you are confusing me BIG TIME!

https://github.com/crysalead-js/dom-layer/blob/master/node/patcher/select-value.js#L10-L15

value are first defined as an argument, then you overwrite it and set the value again, and on the line under there again, you overwrite value again. var value = node.attrs && node.attrs.value; will never be reached.

Couple a lines under there again - https://github.com/crysalead-js/dom-layer/blob/master/node/patcher/select-value.js#L21-L22

Here values are defined two times and overwrite the first one.

And you continue - https://github.com/crysalead-js/dom-layer/blob/master/node/patcher/select-value.js#L46-L47

And this multiple - https://github.com/crysalead-js/dom-layer/blob/master/node/patcher/select-value.js#L10 are never used

jails commented 9 years ago

I removed some useless code in https://github.com/crysalead-js/dom-layer/commit/05e045b62df1af802d23ff1893b813701b4bbbb8 But you can open a PR if you have a better idea ;-)