wdtinc / mapbox-vector-tile-java

Java Mapbox Vector Tile Library for Encoding/Decoding
Apache License 2.0
147 stars 73 forks source link

JtsAdapter is swallowing it's exception #39

Open dadadamarine opened 5 years ago

dadadamarine commented 5 years ago
private static List<Geometry> flatIntersection(Geometry envelope, List<Geometry> dataGeoms) {
        final List<Geometry> intersectedGeoms = new ArrayList<>(dataGeoms.size());

        Geometry nextIntersected;
        for(Geometry nextGeom : dataGeoms) {
            try {

                // AABB intersection culling
                if(envelope.getEnvelopeInternal().intersects(nextGeom.getEnvelopeInternal())) {

                    nextIntersected = envelope.intersection(nextGeom);
                    if(!nextIntersected.isEmpty()) {
                        nextIntersected.setUserData(nextGeom.getUserData());
                        intersectedGeoms.add(nextIntersected);
                    }
                }

            } catch (TopologyException e) {
                LoggerFactory.getLogger(JtsAdapter.class).error(e.getMessage(), e);
            }
        }

        return intersectedGeoms;
    }

I cannot catch TopologyException because of this , and also cannot handle log about this. plz throw exception to us Or just printStackTrace so that we can handle Exception, and it's log

ShibaBandit commented 5 years ago

I agree some custom handling would be nice here. The exception is logged, what is the reason for printStackTrace over logging it?

dadadamarine commented 5 years ago

I think that, using slf4j in library codes means forcing programmer to log this error message, even if they dont want that.

but printStackTrace don't. printStackTrace just show error code in programmer's console, but doesn't write logs on logging file or server console. so programmer can choose one between logging this message on file or not

Of courese, I agree with that custom handling or throwing error to programmer is best. but this way of modification will break backward compatibility also.