You are right for third party subclasses that extends a method or an
interface.
I was only thinking about classical public methods calling, when a caller
calls them, without inheriting them. When it only uses a method of that
class and catches its problems.
They are two troubles currently in Geotools that cause a problem of accuracy
:
1) Many methods tells in their throws directives that they might throw an
exception, and they don't throw it at all. This is really misleading :
"Go on holidays, be confident ! I will warn you if someone try to break into
your house."
"The nuclear power plant ? Don't worry, I am watching it, I warn you if
something happens."
And its not the case.
- Callers rely on you, but you are not checking what you claim checking.
- Every caller have to catch your exception, generating noise in the code,
for something you are in fact not checking at all.
This is the worst thing I know, the false declarations of exceptions.
Everyone should set its Eclipse warning against this and purge the methods
against this, in my mind (settings : errors/warnings unnecessary declaration
of thrown exception).
2) A caller calls our public method and a java.lang.Exception occurs in one
of its sub methods.
Our API says : "We have a problem.", and the end user receive a message such
as these ones, that ends his program :
"CRS:123456"
"Unsupported geometry"
"Incorrect syntax near SIZE"
"Connection refused."
"Data not found"
We provided no help at all and nothing more can be done for him. He can go
the user mailling list and ask for help.
Exceptions should be specialized to in three sets :
- Exceptions that shows that Geotools API is broken and shall be corrected.
"Incorrect syntax near SIZE", for example, if its cause is a wrong internal
SQL statetment used by Geotools for any goal.
- Exceptions that depicts a temporary trouble or an environment trouble :
"connection refused" = Database connection refused with its useful
properties to know which connection has failed and why.
- "business" Exceptions coming from bad data or request done by the caller,
involving the end-user or the caller. Then the exception thrown should be
the most accurate, the more specific, to explain what really happened and
the most exactly it can be interpreted.
-- The exception carrying the "Data not found" message is may be in the
context where it is thrown meaning that a feature the user is searching for
does not exist, and then a better exception should be thrown instead.
-- The exception carrying the message "CRS:123456" should tell that this
CRS does not exist and be a CRSNotFoundException or something like that.
-- "Geometry not supported" message is typical : what geometry ? A
point, a line, a polygon ? Why ? Who gave it ? And where ? in Geotools, this
message can come from 40 different places !
If this is done, Geotools can become more clever, and the user applications
will be able to use them too this more accurate exceptions, avoiding the end
user to receive ends messages stopping its application at anytime. when an
alternative can be choosen because the calling program now understand what
is truly happening.
At least, if a message has to be sent to the end user, a good, accurate and
comprehensive message can be displayed :
from :
catch(Exception e) {
// where e.getMessage() = "Data not found"
}
you can't tell and do nothing.
But from a FeatureNotFoundException given by Geotools and interpreted by
the calling application in its context, the end user may receive a far more
helpfull message.
Doing speacialization of exceptions, purging methods of their unused
exceptions declared but not really thrown, may sometimes lead to backward
compatibily issues that must be checked and discussed.
But the current state is misleading in many places, and until specialization
of exceptions begins, Geotools cannot be fully debugged.
Really, it cannot. Its not possible to do assertions and full tests on
methods that confuse many troubles in a single exception. Because if I enter
some test data to a rather top level method of Geotools, the only way to
understand what happened in case of trouble is to do :
catch(Exception e) {
if (e.getMessage() == null) {
// we are lost
}
if (e.getMessage().startsWith("Unsupported geometry")) {
...
}
if (e.getMessage().startsWith("CRS")) {
...
}
if (e.getMessage().startsWith("CRS")) {
...
}
}
its clumsy, trouble prone, and nothing can be really proven with this.
Refactoring some of the exceptions thrown is needed to eventually succed in
making Geotools working perfectly and be helpful in any circumstances.
Regards,
Marc Le Bihan
-----Message d'origine-----
From: Ben Caradoc-Davies
Sent: Saturday, July 15, 2017 5:40 AM
To: Marc Le Bihan
Cc: geotools-***@lists.sourceforge.net
Subject: Re: [Geotools-devel] Are refactorings of core exceptions permitted
?
Marc,
what about a third-party subclass that extends or implements the method?
This is also a valid use of the GeoTools API. If your refactoring
narrows the type of the throws specification on a method in the public
GeoTools API, every third-party subclass or implementation that
overrides or implements the method will be invalid because it would be
broadening the type of the exception. This violates the Liskov
substitution principle:
https://en.wikipedia.org/wiki/Liskov_substitution_principle
Subclasses or implementations with methods that throw broader exceptions
than the superclass or interface are not permitted in Java.
For example, the following will not compile. In this example, the throws
specification on Foo.foo was refactored from Exception to IOException,
breaking Bar.foo:
import java.io.IOException;
public class ExceptionRefactoringExample {
public class Foo {
public void foo() throws IOException {
};
}
public class Bar extends Foo {
public void foo() throws Exception {
}
}
}
Kind regards,
Ben.
Post by Marc Le Bihan"
And no users of the API will have to do any change.
No touble with backward compatibility then. "
-----Message d'origine----- From: Marc Le Bihan
Sent: Thursday, July 13, 2017 11:40 PM
To: Ben Caradoc-Davies
Subject: Re: [Geotools-devel] Are refactorings of core exceptions
permitted ?
Thanks for your quick reply.
If a specific exception, let say, UnknownCRSException is changed by a core
try {
CRS crs = loadCRS("crs:12345678");
}
catch(UnknownCRSException e) {
// Do something to understand that problem.
}
catch(Exception e) {
// Something has broken, but we don't know what, where and why.
}
There will be a big backward compatibility trouble at the same time of a
regression.
try {
CRS crs = loadCRS("crs:12345678");
}
catch(Exception e) {
// Something has broken, but we don't know what, where and why.
}
because most of the methods throws java.lang.Exception.
public CRS loadCRS() throws Exception
public CRS loadCRS() throws UnknownCRSException
(with UnknownCRSException extends Exception),
try {
CRS crs = loadCRS("crs:12345678");
}
catch(Exception e) {
// Do something to understand that problem.
}
And no users of the API will have to do any change.
No backward compatibility then.
They will gain the choice of catching UnknownCRSException instead of
try {
CRS crs = loadCRS("crs:12345678");
}
catch(UnknownCRSException e) {
// Do something to understand that problem.
}
which is an improvement, because you give for each inexpected event a more
accurate description.
Regards,
Marc.
-----Message d'origine----- From: Ben Caradoc-Davies
Sent: Thursday, July 13, 2017 10:49 PM
To: Marc Le Bihan
Subject: Re: [Geotools-devel] Are refactorings of core exceptions permitted
?
Marc,
code improvements are welcome, but changes to public APIs require change
proposals and are typically accepted only on the master branch. This
includes changes to throws specifications.
Backwards compatibility of both source and binaries is a major concern.
What happens if a throws exception on a public interface is changed to a
narrower type? Can instances of this interface be passed to third-party
code compiled against the old interface?
Exceptions of general utility could be defined in gt-main. Exceptions
that depend on specific module functionality could be defined in that
module.
Kind regards,
Ben.
Post by Marc Le BihanHello,
During the next years, I might have to work on Geotools and Apache
SIS. (I already commited on Apache SIS on a JDBC driver for shapefiles).
When I look geotools code, I sometimes think about things that could
make it stronger. I have a fork ready, but I won’t start submitting
May I exchange java.lang.Exception that I see most of the times on
the throws directive of geotools methods to more specialized exceptions ?
This would allow the caller of these methods to perform, if he
catch(Exception e) {
...
}
he can do today, and that cannot be handled by the program flow
because it says : “Something has failed, but I can’t tell you what and
where.”.
I think that debugging some parts of geotools and improving help
given by its responses involves changing the exceptions thrown in some
places.
If creating more specific exceptions to replace the core ones is
encouraged, where these new exceptions should be created ?
- In the same package of the class where they will be thrown ?
- In a specfic package of the gt-[module] ?
Regards,
Marc Le Bihan
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
GeoTools-Devel mailing list
https://lists.sourceforge.net/lists/listinfo/geotools-devel
--
Ben Caradoc-Davies <***@transient.nz>
Director
Transient Software Limited <http://transient.nz/>
New Zealand