Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception class cleanup #5

Closed
secondtruth opened this issue Jan 7, 2012 · 6 comments
Closed

Exception class cleanup #5

secondtruth opened this issue Jan 7, 2012 · 6 comments

Comments

@secondtruth
Copy link
Contributor

Do we really need so many Exception classes? Can we merge (or even delete) some of them?

Due to #1 and #3 we have to additionally split them into separate files, so it's good if we don't have so many classes.

@LinuxDoku
Copy link
Member

+1, the error level can be also handled inside of one class.

Less is more ;-)

@jnugh
Copy link
Contributor

jnugh commented Jan 7, 2012

Not quite, the more exceptions, the more possibilities to catch just the exceptions one wants to catch.
So in this case, more is more flexible. Look at javas inbuild exceptions for example.

@secondtruth
Copy link
Contributor Author

You could do this:

class LitotexException extends Exception {
    const ERROR = 2;
    const WARNING = 1;
    private $_type;
    public function __construct($message, $type = self::WARNING) {
        // ...
    }
    public function getType() {
        return $this->_type;
    }
}

throw new LitotexException('some message', LitotexException::ERROR);

if ($e->getType() == LitotexException::ERROR) {
    echo 'It is an error!';
}

@jnugh
Copy link
Contributor

jnugh commented Jan 8, 2012

But the way Exceptions should be used is a lot more intuitive.
So I would need a switch case instead of several catch clauses. Its just not the way Exceptions should be used, you need to write a lot more to handle all types and usual try catch blocks with more than one catch clause are better to read than switch case in one catch block.
To be serious the few Exceptions which are existing in Litotex are too less, we need more.
Maybe we could import StandardPHPLibrary files, or just extend current Exceptions.
Anyway, deleting current Exception classes is a bad idea! Definitive.

( http://www.php.net/~helly/php/ext/spl/classException.html is a good Exceptions structure, maybe a little overhead. I will try to find better solutions later.)

@secondtruth
Copy link
Contributor Author

But the way Exceptions should be used is a lot more intuitive. (...)

+1

Maybe we could import StandardPHPLibrary files, or just extend current Exceptions.

That's a good compromise.


  • Why does lttxError::__construct() begin with $args = func_get_args(); $messageCode = $args[0];?
  • What does lttxInfo do?
  • Why does lttxFatalError extend Exception and not lttxError?

@jnugh
Copy link
Contributor

jnugh commented Jan 8, 2012

lttxError can replace %s and similar in language strings.
lttxInfo is only for debugging. Dunno if used very often but you can just throw it when you need a stack trace (DEVDEBUG must be true).
Third, if you want to catch lttxError but not lttxFatalError, they must both be children of Exception rather than another custom exception class.

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

No branches or pull requests

3 participants