Library Form_validation ¿bug?

[bcit-ci]

Library Form_validation ¿bug?

[stickgrinder]

I think this should throw an exception instead than just return false. In fact it is not that provided data doesn't match validation, but it's the software that's broken since a component is missing.
I could bear with this since I'm working on form components.

[stickgrinder]

I took a look into Form_validation library and current behavior is to log a debug message for non existent function without breaking execution.
It is not that desirable, actually, since this weaken validation, that should be strengthen instead. Someone from Reactor team on this?

[kenjis]

I agree to throwing an exception. This current spec is too dangerous.

[freewil]

+1, throw exception.

[kenjis]

@freewll, your hack does not throw an exception.

When this error happens, it is a bug. So I think error message for users is not required, only throwing an exception is enough. If you does not catch the exception, PHP fatal error occurs.

[freewil]

I don't think an exception is appropriate considering the current framework. I'm not aware of anywhere else in CI where an exception is thrown.

[stickgrinder]

Actually I checked the core and exceptions are not the framework preferred instrument to raise errors. Still if I understand correctly, your fix simply logs an error message and nothing more.

It would be better to use show_error() helper, that mimic exceptions handling while remaining compatible with better error reporting tools such as Uh-Oh. As Kenjis pointed out, trying to call a non existent function should result in execution stop, both in framework and in language philosophy! :)

HTH.

[stickgrinder]

FYI, here is a brief post that explains advantages of using CI's core error handling: http://www.derekallard.com/blog/post/error-handling-in-codeigniter

My bad, I thought to an Exception, but you're right: CI doesn't advocate exceptions. show_error() should be the right way to go. :)

[kenjis]

You guys are right. But the article was written in 2007!
CI had supported PHP4 until before CI2.0, so there was no Exception, try/catch/throw.

If we throw Exception, we need to hear the policy about Exception from Reactor team and/or EllisLab.

[stickgrinder]

Well, actually this is an issue with a lot of topics. Since last steering from Reactor team (all the github migration, Core and Reactor merging and various stuff) it seems a lot could change. But as far as I know there is no intention to drastically change the way CI behaves (that's one of the reasons that led to Fuel creation: they didn't feel changing the mindset of CI was right), so as far as I can tell, if coding guidelines are unavailable, the only thing to do is clone the repo and grep for "throw", "try" and "except" in system directory.

If nothing can be found, well, my bet is nothing is changed since 2007 on that side! ;)

[freewil]

I closed my pull request and agree with @stickgrinder that show_error() should be used instead.

[stickgrinder]

I'll try to send a pull-req ASAP with a show_error() integration.