Create a new exception type for unsupported operations?

[aehimself]

A couple of times I already had to determine if .Ping failed with an exception or is not supported at all. At the moment there's no other way but to check the exception class and compare the message as string.

Do you think it'd break things if we would introduce a new, EZUnsupportedException type? We would't have to worry about localization anymore, as it would be sufficient to check the exception class.

I can make the change, just wanted to ask your opinion before I do so.

[aehimself]

...if we inherit it from EZSQLException, no previous code should break

[Fr0sT]

I'd say Ping should return boolean flag and only raise exception when something really bad happens

[aehimself]

If a method is not implemented in the driver and / or not supported by the client library, we throw an exception:

Code: Select all

function TZAbstractDbcConnection.PingServer: Integer;
begin
  Result := 1;
  RaiseUnsupportedException;
end;

Which is implemented like:

Code: Select all

procedure TZAbstractDbcConnection.RaiseUnsupportedException;
begin
  raise EZSQLException.Create(SUnsupportedOperation);
end;

I personally agree with this way - if for example .Ping returns false it means that the server did not reply; if pinging is not supported it should be distinguished from this result.

I'm just looking for a fool-proof way to identify unsupported operations.

Edit: Being not supported is not bad enough?

[marsupilami]

Maybe we should extend IZDatabaseInfo with a SupportsPing method?

[Fr0sT]

Maybe we should extend IZDatabaseInfo with a SupportsPing method?

Does it have any sense? Any server should be "pingable" via executing an empty query. And, BTW, Ping for FB and Pg doesn't re-establish the connection which is inconsistent to MySQL implementation

[marsupilami]

Maybe we should extend IZDatabaseInfo with a SupportsPing method?

Does it have any sense? Any server should be "pingable" via executing an empty query. And, BTW, Ping for FB and Pg doesn't re-establish the connection which is inconsistent to MySQL implementation

They should be pingable - yes. But as long as it is not implemented on the DBC layer, generic programs could check using the proposed IZDatabaseInfo.SupportsPing.

[aehimself]

Maybe we should extend IZDatabaseInfo with a SupportsPing method?

I am strongly against it.

The reason I started to think about this was the implementation of .AbortConnection, I just brought .Ping as an example. With this idea, we should create a Supports* to every method possible. This would be a huge and unnecessary work, which later might need adjusting if changes were made.
Plus, what to return for Supports*, if an earlier version of client library does not support a specific operation (thinking on your FireBird adjustment)? Yes, we could hardcode version numbers but that's something I really, really don't like.

I think raising an exception (like it is now) is the cleanest and easiest way to handle these; I just think it would be cleaner if there would be a separate exception class for not supported operations. So instead of:

Code: Select all

If (E Is EZSQLException) And (E.Message = 'Unsupported operation')

... which will not work with localizaions, we simply could verify this by

Code: Select all

If (E Is EZUnsupportedException)

Plus, if EZUnsupportedException is defined as

Code: Select all

EZUnsupportedException = Class(EZSQLException)

no previous code should break due to (E Is EZSQLException) still should return true.

[...] generic programs could check using the proposed IZDatabaseInfo.SupportsPing.

The way I handled this is I introduced a boolean, named "shouldping" with the initial value of True. Right next to the elapsed time between the last ping and Now I checked if we "shouldping" and that's when I called .Ping. In the exception handler if the error was unsupported, I simply toggled "shouldping" to false. Foolproof

Any server should be "pingable" via executing an empty query.

Deja-vu. Like if I was told it's a bad practice
http://zeoslib.sourceforge.net/viewtopi ... 99#p106099

[Fr0sT]

Plus, what to return for Supports*, if an earlier version of client library does not support a specific operation (thinking on your FireBird adjustment)? Yes, we could hardcode version numbers but that's something I really, really don't like.

Version-dependent stuff will remain anyway... of course, we try to avoid such conditionals where possible.

Deja-vu. Like if I was told it's a bad practice

Ha-ha, and I am rather like "Whoa was that me who wrote this"
Not a bad practice until pinging happens 100 times/sec, and only for those drivers who lack other methods.

[aehimself]

Version-dependent stuff will remain anyway... of course, we try to avoid such conditionals where possible.

Not necessarily. I really like Jan's correction, I'm thinking on modifying all the other implementations to use this method (except; throw an exception, not to return 1):

Supported.PNG

[Fr0sT]

Not necessarily. I really like Jan's correction, I'm thinking on modifying all the other implementations to use this method (except; throw an exception, not to return 1):

Sure, this method is what I meant with "avoid where possible". But there are things that couldn't be checked by availability of a function in client lib, that is, server capabilities or function parameter change.

[aehimself]

So, what about the new exception class?

I would also like to get rid of the RaiseUnsupportedException method, as now we have like 3 versions doing the same thing, plus 10 places simply raising the exception itself

[Fr0sT]

Personally I'm fine with new exc class as well as replacing the method, but regarding the method, Jan seems to have strong objection

[aehimself]

Don't get me wrong: I don't have any problems with .Supports* other than the (to me, that is) seemingly unnecessary, mostly administrative load. If it is going to be implemented and will be kept up to date I'll even consider updating my prehistoric codes to use them. With that said - if I'd have to maintain a codebase as large as Zeos I would do anything to avoid this.

Maybe we should just wait for @Egonhugeist, as so far it seems to be 1 against, 1 supporting, 1 abstaining

I'll keep this idea in my stash until then.

[marsupilami]

Hello,

but regarding the method, Jan seems to have strong objection

I have strong objections all the time - don't take that too seriously

In my opinion it is bad design, if the only way to find out if something is supported, is to try to use it and fail. The original question is:

A couple of times I already had to determine if .Ping failed with an exception or is not supported at all. At the moment there's no other way but to check the exception class and compare the message as string.

In my opinion Zeos should provide the information wether it makes sense to use the ping method at all. If you know that Ping is not supported, you don't even need to try. And if you try, you will know that an exception means a real failure to execute the Ping. You don't even need to check the exception class.

Best regards,

Jan