Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#3860] - [#33928] Minor code improvement

? Error

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
7 Jul 2014

Reduce if- else on function countMenuChildren()
Reduce if-else on function render()

tracker:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33928

avatar dgt41 dgt41 - open - 7 Jul 2014
avatar dgt41
dgt41 - comment - 7 Jul 2014

Travis is failing in 5.3 with msg:

Sorry, phpunit/DbUnit references an unknown channel pear.symfony.com for pear.symfony.com/Yaml
Do you want to add this channel and continue?
Please choose:
  yes
  no
avatar Bakual
Bakual - comment - 7 Jul 2014

I agree with the first change.
The second one I just don't care much. Imho it's not really better to initialise a variable with a value and overwrite it in the next step. An if-else would be appropriate here. I'm fine with both variants.
Can you add an empty line before the first return and the second if? That would make our code style checks happy.

avatar dgt41 dgt41 - change - 7 Jul 2014
Title
…
Minor code improvement
[#33928] Minor code improvement
avatar dgt41
dgt41 - comment - 7 Jul 2014

I missed that enter :)
I think that defining a var and then change the value is not a bad strategy, in this case is less lines, if that’s a good metric...

avatar Bakual
Bakual - comment - 7 Jul 2014

I think that defining a var and then change the value is not a bad strategy, in this case is less lines, if that’s a good metric...

I'd say it is neither worse nor better :smile:

avatar wilsonge
wilsonge - comment - 8 Jul 2014

I personally prefer both the changes here.

@test works as expected

avatar losedk
losedk - comment - 9 Jul 2014

Tested. Works fine!

avatar Bakual Bakual - change - 9 Jul 2014
Status New ⇒ Closed
Closed_Date 0000-00-00 00:00:00 ⇒ 2014-07-09 18:48:45
avatar Bakual Bakual - close - 9 Jul 2014

Add a Comment

Login with GitHub to post a comment