Page 1 of 1

Raise Exeption.Create

Posted: 18.11.2021, 12:15
by aehimself
Hello,

I was going through the source code to raise EZUnsupportedExceptions where necessary and did a quick search.
" Raise Exception.Create"
appears at 101 places inside Zeos, and raising the base Exception object directly is never a good idea.

What do you think if we replace all normal exceptions with EZSQLException?

Would fit the suite a little bit better I think.

Re: Raise Exeption.Create

Posted: 18.11.2021, 13:12
by marsupilami
I fully agree :)

Re: Raise Exeption.Create

Posted: 18.11.2021, 13:44
by aehimself
After the replacement, the following units had to import ZDbcIntFs as they had no idea what EZSQLException is:
- ZSysUtils (Core)
- ZClasses (Core)
- ZPlainLoader (Plain)
- ZVariables (Core)
- ZDbcProxyUtils (DBC, this is fine)
- ZScriptParser (ParseSQL)

If I make the checkin like this, Core, Plain and ParseSQL will reach to an upper package (Dbc), which is also not ideal.

The definition of EZSQLThrowable and EZSQLException have to me moved back to Core if we want to keep the structuring. Keep in mind that EZSQLThrowable can be inherited from EDatabaseError, maybe this is why it's at DBC?

Code: Select all

  EZSQLThrowable = class({$IFDEF DO_NOT_DERIVE_FROM_EDATABASEERROR}Exception{$ELSE}EDatabaseError{$ENDIF})
Thoughts about this? Go / no-go? Any particular "base" typedef unit I can use or just create a new one (ZExceptions)?

Re: Raise Exeption.Create

Posted: 18.11.2021, 17:44
by marsupilami
Well - this touches a problem where Egonhugeist and me disagree - a lot. I think that Zeos exceptions should be implemented in ZCore and that they should inherit from EDatabaseError. Egonhugeist thinks that Exceptions should not derive from EDatabaseError because some people seem to think that pulling the DB unit into their programs gives them bad Karma or something like that. I never understood this. Sorry if this part of the post contains sarcasm. I can't help it.

Soooo - what to say - from my POV we should move these exceptions and use them. People already are able to decide if they want to derive from EDatabaseError or not. Exceptions should be declared in core units, so they can be used consistently throughout all Zeos code.

Re: Raise Exeption.Create

Posted: 18.11.2021, 18:34
by aehimself
In this case I'll simply move exception definitions in a separate unit under Core.

I too do believe that that is their place.

Re: Raise Exeption.Create

Posted: 19.11.2021, 09:15
by aehimself
I hope no manual work is involved when patches are applied... about 80 files had to be changed :)

Fresh and crispy pull request available on GitHub.
I also added the missing Xml reference to DBC as it made my command line compilation to fail.

Let's see what Jenkins will say.

Re: Raise Exeption.Create

Posted: 19.11.2021, 17:24
by marsupilami
Applied ;)

Re: Raise Exeption.Create

Posted: 19.11.2021, 17:52
by aehimself
Damn, the test suite... it was in my mind to doublecheck the uses clauses in them too but then the package didn't want to compile and while I was fixing that I forgot about it...

Thanks for fixing them up!