zc_install - for issue #99

<a href='#crh-start'></a><a href='#crh-data-%7B%22processed%22%3A%20%5B%22https%3A//github.com/zencart/zc-v1-series/pull/100%23issuecomment-49129763%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23issuecomment-49129971%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23issuecomment-49164964%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23issuecomment-49188753%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23issuecomment-49256109%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23issuecomment-49256249%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23issuecomment-49256362%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14977316%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14977318%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14977323%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14977365%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14978240%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14978328%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14981176%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r15008192%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r15008194%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r15008195%22%5D%2C%20%22comments%22%3A%20%7B%22General%20Comment%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23issuecomment-49129763%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Not%20a%20huge%20fan%20of%20multiple%20exit%20points%20for%20functions%20/%20methods.%20Just%20my%20opinion%2C%20but%20I%20cringe%20when%20I%20see%20more%20than%20two%20exit%20points%20%28excluding%20exceptions%29.%20Suppose%20it%20is%20more%20of%20a%20style%20preference...%5Cr%5Cn%5Cr%5CnHowever%2C%20I%20feel%20some%20of%20the%20longer%20methods%20%28with%20multiple%20exit%20points%29%20would%20be%20good%20candidates%20for%20refactoring%20into%20multiple%20smaller%20methods%20in%20the%20future.%22%2C%20%22created_at%22%3A%20%222014-07-16T06%3A51%3A47Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/2963958%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/lhungil%22%7D%7D%2C%20%7B%22body%22%3A%20%22Excited%20to%20see%20an%20upgrade%20path%20from%20pre%201.5%21%22%2C%20%22created_at%22%3A%20%222014-07-16T06%3A54%3A38Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/2963958%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/lhungil%22%7D%7D%2C%20%7B%22body%22%3A%20%22%40lhungil%20I%20agree%2C%20multiple%20return%20points%20are%20an%20indication%20that%20refactoring%20is%20needed%2C%20it%27s%20not%20just%20style.%20%202%20returns%20is%20probably%20a%20good%20maximum.%22%2C%20%22created_at%22%3A%20%222014-07-16T13%3A29%3A48Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22Ya%2C%20%40zcwilt%20and%20I%20were%20thinking%20the%20same%20thing%20when%20we%20were%20troubleshooting%20a%20quirk%20in%20there%20yesterday.%20Definitely%20something%20to%20tackle%20in%20the%20future.%20We%20opted%20to%20wait%20since%20zc_install%20is%20used%20once%20at%20install%20time%20and%20then%20deleted.%20Doesn%27t%20have%20to%20be%20stellar%20code%2C%20so%20doesn%27t%20really%20require%20our%20immediate%20focus.%20A%20good%20project%20for%20when%20other%20priorities%20aren%27t%20competing.%22%2C%20%22created_at%22%3A%20%222014-07-16T16%3A09%3A37Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/404472%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/drbyte%22%7D%7D%2C%20%7B%22body%22%3A%20%22agreed%2C%20that%20we%20definitely%20need%20some%20re-factoring%2C%20The%20upgradeSql%20code%20in%20particular%20would%20benefit.%5Cr%5Cnapart%20from%20that%20though%20%5Cr%5Cn%3A%2B1%3A%20%22%2C%20%22created_at%22%3A%20%222014-07-17T03%3A59%3A54Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/227354%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/zcwilt%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%22%2C%20%22created_at%22%3A%20%222014-07-17T04%3A03%3A37Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/2963958%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/lhungil%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3Ashipit%3A%22%2C%20%22created_at%22%3A%20%222014-07-17T04%3A06%3A18Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1992427%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/ajeh%22%7D%7D%5D%2C%20%22title%22%3A%20%22General%20Comment%22%7D%2C%20%22Pull%20a522f0ea17a4a7632ddb5f93153206f3c5cab21a%20zc_install/includes/classes/class.systemChecker.php%2046%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14977323%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%5BUnnecessary%5D%28http%3A//c2.com/cgi/wiki%3FElseConsideredSmelly%29%20%60else%60.%22%2C%20%22created_at%22%3A%20%222014-07-16T01%3A09%3A57Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%3Ca%20href%3D%27%23crh-update-none%27%3E%3C/a%3E%22%2C%20%22created_at%22%3A%20%222014-07-16T16%3A09%3A54Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/404472%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/drbyte%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20zc_install/includes/classes/class.systemChecker.php%3AL525-565%22%7D%2C%20%22Pull%20a522f0ea17a4a7632ddb5f93153206f3c5cab21a%20zc_install/includes/languages/lngEnglish.php%2047%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14977316%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%3A%2B1%3A%20%22%2C%20%22created_at%22%3A%20%222014-07-16T01%3A09%3A43Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20zc_install/includes/languages/lngEnglish.php%3AL140-147%22%7D%2C%20%22Pull%20a522f0ea17a4a7632ddb5f93153206f3c5cab21a%20zc_install/includes/classes/class.systemChecker.php%2010%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14977365%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22I%20don%27t%20really%20like%20this%20complex%20conditional%20-%20but%20at%20the%20very%20least%2C%20you%20can%20%60return%20TRUE%3B%60%20here.%22%2C%20%22created_at%22%3A%20%222014-07-16T01%3A12%3A14Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%3Ca%20href%3D%27%23crh-update-none%27%3E%3C/a%3E%22%2C%20%22created_at%22%3A%20%222014-07-16T16%3A09%3A54Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/404472%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/drbyte%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20zc_install/includes/classes/class.systemChecker.php%3AL231-243%22%7D%2C%20%22Pull%20a522f0ea17a4a7632ddb5f93153206f3c5cab21a%20zc_install/includes/classes/class.zcDatabaseInstaller.php%2017%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/100%23discussion_r14977318%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22If%20this%20isn%27t%20being%20used%2C%20it%20can%20be%20removed.%20%20Source%20control%20will%20keep%20track%20of%20it.%20%20Otherwise%2C%20please%20condense%20it.%22%2C%20%22created_at%22%3A%20%222014-07-16T01%3A09%3A50Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22I%20was%20just%20working%20on%20this.%20There%27s%20actually%20a%20bug%20here%20in%20that%20other%20parts%20of%20the%20code%20are%20expecting%20a%20return%20value.%20Some%20of%20that%27s%20legacy%20logic%2C%20and%20warrants%20turning%20into%20another%20issue%20to%20address%20that.%22%2C%20%22created_at%22%3A%20%222014-07-16T01%3A56%3A54Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/404472%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/drbyte%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%20Carry-on%2C%20then.%20Wasn%27t%20sure%20if%20you%20had%20finished%20yet.%22%2C%20%22created_at%22%3A%20%222014-07-16T02%3A00%3A36Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22Updated.%20Works%20better%20now.%22%2C%20%22created_at%22%3A%20%222014-07-16T04%3A48%3A23Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/404472%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/drbyte%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%3Ca%20href%3D%27%23crh-update-complete%27%3E%3C/a%3E%22%2C%20%22created_at%22%3A%20%222014-07-16T16%3A09%3A55Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/404472%3F%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/drbyte%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20zc_install/includes/classes/class.zcDatabaseInstaller.php%3AL91-104%22%7D%7D%2C%20%22approved%22%3A%20%7B%22https%3A//github.com/ajeh%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1992427%3F%22%7D%7D%7D'></a>
<a href='https://www.codereviewhub.com/'><img src='http://www.codereviewhub.com/site/github-bar.png' height=40></a>

<img src='http://www.codereviewhub.com/site/github-approved-avatar.png'><a href='https://github.com/ajeh'><img src='https://avatars.githubusercontent.com/u/1992427?size=34' width=34 height=34></a>

- [x] <a href='#crh-comment-Pull a522f0ea17a4a7632ddb5f93153206f3c5cab21a zc_install/includes/classes/class.zcDatabaseInstaller.php 17'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/100#discussion_r14977318'>File: zc_install/includes/classes/class.zcDatabaseInstaller.php:L91-104</a></b>
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?size=16' height=16 width=16'></a> If this isn't being used, it can be removed.  Source control will keep track of it.  Otherwise, please condense it.
- <a href='https://github.com/drbyte'><img border=0 src='https://avatars.githubusercontent.com/u/404472?size=16' height=16 width=16'></a> I was just working on this. There's actually a bug here in that other parts of the code are expecting a return value. Some of that's legacy logic, and warrants turning into another issue to address that.
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?size=16' height=16 width=16'></a> :+1: Carry-on, then. Wasn't sure if you had finished yet.
- <a href='https://github.com/drbyte'><img border=0 src='https://avatars.githubusercontent.com/u/404472?size=16' height=16 width=16'></a> Updated. Works better now.
- [x] <a href='#crh-comment-Pull a522f0ea17a4a7632ddb5f93153206f3c5cab21a zc_install/includes/classes/class.systemChecker.php 46'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/100#discussion_r14977323'>File: zc_install/includes/classes/class.systemChecker.php:L525-565</a></b>
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?size=16' height=16 width=16'></a> [Unnecessary](http://c2.com/cgi/wiki?ElseConsideredSmelly) `else`.
- [x] <a href='#crh-comment-Pull a522f0ea17a4a7632ddb5f93153206f3c5cab21a zc_install/includes/classes/class.systemChecker.php 10'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/100#discussion_r14977365'>File: zc_install/includes/classes/class.systemChecker.php:L231-243</a></b>
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?size=16' height=16 width=16'></a> I don't really like this complex conditional - but at the very least, you can `return TRUE;` here.
- [x] <a href='#crh-comment-General Comment'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/100#issuecomment-49129763'>General Comment</a></b>
- <a href='https://github.com/lhungil'><img border=0 src='https://avatars.githubusercontent.com/u/2963958?size=16' height=16 width=16'></a> Not a huge fan of multiple exit points for functions / methods. Just my opinion, but I cringe when I see more than two exit points (excluding exceptions). Suppose it is more of a style preference...
However, I feel some of the longer methods (with multiple exit points) would be good candidates for refactoring into multiple smaller methods in the future.
- <a href='https://github.com/lhungil'><img border=0 src='https://avatars.githubusercontent.com/u/2963958?size=16' height=16 width=16'></a> Excited to see an upgrade path from pre 1.5!
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?size=16' height=16 width=16'></a> @lhungil I agree, multiple return points are an indication that refactoring is needed, it's not just style.  2 returns is probably a good maximum.
- <a href='https://github.com/drbyte'><img border=0 src='https://avatars.githubusercontent.com/u/404472?size=16' height=16 width=16'></a> Ya, @zcwilt and I were thinking the same thing when we were troubleshooting a quirk in there yesterday. Definitely something to tackle in the future. We opted to wait since zc_install is used once at install time and then deleted. Doesn't have to be stellar code, so doesn't really require our immediate focus. A good project for when other priorities aren't competing.
- <a href='https://github.com/zcwilt'><img border=0 src='https://avatars.githubusercontent.com/u/227354?size=16' height=16 width=16'></a> agreed, that we definitely need some re-factoring, The upgradeSql code in particular would benefit.
apart from that though
:+1:


<a href='https://www.codereviewhub.com/zencart/zc-v1-series/pull/100?mark_as_completed=1'><img src='http://www.codereviewhub.com/site/github-mark-as-completed.png' height=26></a>&nbsp;<a href='https://www.codereviewhub.com/zencart/zc-v1-series/pull/100?approve=1'><img src='http://www.codereviewhub.com/site/github-approve.png' height=26></a>&nbsp;<a href='https://www.codereviewhub.com/zencart/zc-v1-series/pull/100?approve=0'><img src='http://www.codereviewhub.com/site/github-undo-approve.png' height=26></a>&nbsp;<a href='https://github.com/zencart/zc-v1-series/pull/100'><img src='http://www.codereviewhub.com/site/github-refresh.png' height=26></a>
<a href='#crh-end'></a>