Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Clarified error messages of $active_group value (error msgs in DB.php) #2040

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants

Used the load->model wrong (had 3rd param as class alias, not second) and spent 30 minutes tracking down the "You have specified an invalid database connection" error rendered from DB.php.
I modified the code to have two different error messages, one unset $active_group, and the other to display the active_group it was looking for.

Gregory T. Corrigan added some commits Nov 29, 2012

Gregory T. Corrigan Clarified error messages of $active_group value 5dc445b
Gregory T. Corrigan Code Formatting tweak; signoff
Signed-off-by: Gregory T. Corrigan <corrigang@taggert.(none)>
7c643c7
Contributor

narfbg commented Nov 29, 2012

Mentioning the $active_group var is a good improvement, but I don't think that printing it's value if it exists justifies adding a few more lines to the code. Actually, the check should be shortened:

if ( ! isset($active_group, $db[$active_group]))

Your call, boss!

Clarity versus speed.

I had intentionally separated the check to have explicit messages for the
two cases.

I would not have found my bug if I had not rendered out what CI though the
$active_group value was; I had $active_group set correctly in my
config.php, but it was being overwritten (accidentally - my bug) in my
model's constructor.

I think we could keep the single if, and still add the value of
$active_group to the error message, solving the issue without adding any
lines to the existing code base.

-Greg

On Thu, Nov 29, 2012 at 12:46 PM, Andrey Andreev
notifications@github.comwrote:

Mentioning the $active_group var is a good improvement, but I don't think
that printing it's value if it exists justifies adding a few more lines to
the code. Actually, the check should be shortened:

if ( ! isset($active_group, $db[$active_group]))


Reply to this email directly or view it on GitHubhttps://github.com/EllisLab/CodeIgniter/pull/2040#issuecomment-10858377.

Gregory T. Corrigan
Dynatics, LLC

http://www.dynatics.com

Contributor

narfbg commented Dec 1, 2012

Okay, agreed then. But you will have to update your code to meet the styleguide and please use parenthesis and not square brackets around the group name.

Also, you'll have to change your text editor/IDE settings to not insert an extra new line at the end of file.

Contributor

narfbg commented Jan 29, 2013

@corrigang Any chance that you could update this PR?

@narfbg narfbg closed this Jul 18, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment