On Thu, Jan 8, 2009 at 9:23 PM, Markus Wiederkehr <[email protected]> wrote: > On Thu, Jan 8, 2009 at 8:44 PM, Robert Burrell Donkin (JIRA) > <[email protected]> wrote: >> >> [ >> https://issues.apache.org/jira/browse/MIME4J-90?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12662087#action_12662087 >> ] >> >> Robert Burrell Donkin commented on MIME4J-90: >> --------------------------------------------- >> >> I don't seem to have done a very good job of explaining the issue. >> >> Using runtime exceptions make things difficult for correct protocol >> implementations. Typically, protocols have their own in-protocol error >> reporting systems. It is expected that exceptions will be checked so that >> developers can caught and correctly handle all expected known issues. >> Runtime exceptions are assumed to be fatal to the session and will typically >> result in the socket disconnecting (for security reasons, it's unwise to >> continue after a programming error). > > Just to make sure I understand your concern correctly: > > You mean protocol implementations that invoke Field.parse() directly? > Because if the protocol implementation uses a MimeStreamParser this > cannot be an issue because AbstractEntity makes sure that the > precondition of Field.parse() does not get violated. > >> These are public classes and form a public API. The runtime was replaced by >> a checked exception to allow these classes to be safely used in protocol >> work in a standalone fashion. > > Again, standalone as in "not driven by MimeStreamParser/ContentHandler"?
Yes Field.parse is public and so is part of the public API (though perhaps this should be reconsidered) >>Reverting this design decision is a potentially dangerous decision since >>protocol implementations may be relying on the checked exception to correctly >>handle that condition. Upgrading will break any protocols which made this >>assumption. >> >> When breaking the contract of a public API, it is better to do this in a >> cleanly incompatible way. If this is not possible then it must be >> highlighted in the release notes. > > What would you think about keeping the checked exception in > Field.parse() and adding a second method that throws an > IllegalArgumentException instead? That second method could be used for > creating or manipulating messages. Because for this use case the > checked exception is really a PITA. that was one of my suggestions, or alternatively just rename parse without replacement so that it's clear the contract has change. but maybe it would be better to quickly reconsider the general design in this area (i'm not a fan) if the general design is retained (i'd be happy enough to see it revised without regard to compatibiltiy), these questions might be worth thinking about: 1. should the parsing methods be part of the general public API? 2. if so, most Field subclasses store ParseException (in a property) when something goes wrong. would it be worthwhile adopting this into the general contract? - robert --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
