Velocity
  1. Velocity
  2. VELOCITY-659

Remove throwing Exception from method signatures

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.7
    • Fix Version/s: None
    • Component/s: Engine
    • Labels:
      None

      Description

      Remove throwing java.lang.Exception from Velocity methods

        Activity

        Hide
        Nathan Bubna added a comment -

        Hmm. Your change in r729843 is both incomplete (things like VelocityEngine were overlooked), and it makes it so that extensions like VelocityTools will not compile against 1.7-dev anymore. Gump will be complaining about this a lot.

        I'm not so sure it's really worth breaking API compatibility for things like LogChute implementations just to clean this up. In a 2.x branch, this would be great, but it's a bit unprecedented to break compatibility like this in Velocity's 1.x series.

        I'm inclined to veto this change, but i'm still open to being convinced otherwise, if you can.

        Show
        Nathan Bubna added a comment - Hmm. Your change in r729843 is both incomplete (things like VelocityEngine were overlooked), and it makes it so that extensions like VelocityTools will not compile against 1.7-dev anymore. Gump will be complaining about this a lot. I'm not so sure it's really worth breaking API compatibility for things like LogChute implementations just to clean this up. In a 2.x branch, this would be great, but it's a bit unprecedented to break compatibility like this in Velocity's 1.x series. I'm inclined to veto this change, but i'm still open to being convinced otherwise, if you can.
        Hide
        Byron Foster added a comment -

        I reversed out the LogChute changes since I don't want to deal with package dependency, and this is a rather obscure API.

        I added the changes to VelocityEngine, I didn't know it existed, but I like it.

        I wonder if there is going to be a 1.7? It may be to soon to answer this, but I'm thinking probably not. However, removing Exception from the method signatures would not break existing Velocity integrations since all references must catch Exception, or fall within a method that throws Exception.

        The one caveat to this is that in VelocityEngine I removed throwing IOException. This would break any code that explicitly catches IOException, you can beat me up for this, but I doubt anyone is being so selective.. If there are any knifes thrown at me I'll change IOException back, otherwise, I'll do it for Velocity.java also. My motives are first to make the Velocity API use unchecked exceptions, which seems like there has already been movement in this direction. The second is that in the case of evaluate it seems unreasonable to force someone to catch an Exception that may or may not occur depending on there usage, for example passing in a StringWriter. Anyway, that's my position, but I know these things have a way generating opinions.

        Show
        Byron Foster added a comment - I reversed out the LogChute changes since I don't want to deal with package dependency, and this is a rather obscure API. I added the changes to VelocityEngine, I didn't know it existed, but I like it. I wonder if there is going to be a 1.7? It may be to soon to answer this, but I'm thinking probably not. However, removing Exception from the method signatures would not break existing Velocity integrations since all references must catch Exception, or fall within a method that throws Exception. The one caveat to this is that in VelocityEngine I removed throwing IOException. This would break any code that explicitly catches IOException, you can beat me up for this, but I doubt anyone is being so selective.. If there are any knifes thrown at me I'll change IOException back, otherwise, I'll do it for Velocity.java also. My motives are first to make the Velocity API use unchecked exceptions, which seems like there has already been movement in this direction. The second is that in the case of evaluate it seems unreasonable to force someone to catch an Exception that may or may not occur depending on there usage, for example passing in a StringWriter. Anyway, that's my position, but I know these things have a way generating opinions.
        Hide
        Nathan Bubna added a comment -

        Thanks, Byron. That should make Gump and me happier. And yeah, i also doubt that anyone was really specifically watching for IOException. Even if they are, i think that's a fine thing to do in a 1.x release. Just be sure to have it clearly documented in the change log, as it is an API change.

        Show
        Nathan Bubna added a comment - Thanks, Byron. That should make Gump and me happier. And yeah, i also doubt that anyone was really specifically watching for IOException. Even if they are, i think that's a fine thing to do in a 1.x release. Just be sure to have it clearly documented in the change log, as it is an API change.
        Hide
        Byron Foster added a comment -

        java.lang.Exception is still thrown from the plugin APIs such as LogChute and EventHandlers, but this at least removes them from most user's point of view. We can address the plugin API when we determine what level of changes are appropriate for the next release.

        Show
        Byron Foster added a comment - java.lang.Exception is still thrown from the plugin APIs such as LogChute and EventHandlers, but this at least removes them from most user's point of view. We can address the plugin API when we determine what level of changes are appropriate for the next release.
        Hide
        Will Glass-Husain added a comment -

        Does that mean that plugins implementing the existing interface no longer work? To me, that breaks backwards compatibility and is a bad idea.

        WILL

        Show
        Will Glass-Husain added a comment - Does that mean that plugins implementing the existing interface no longer work? To me, that breaks backwards compatibility and is a bad idea. WILL
        Hide
        Byron Foster added a comment - - edited

        No, this doesn't change the plugin APIs. when I say "User's point of view" I mean the APIs users work with most directly, mainly Velocity.java and VelocityEngine.java. To be clear:

        • methods in Velocity.java and VelocityEngine.java no longer throw java.lang.Exception and java.io.IOException
        • Plugin API is unchanged.
        Show
        Byron Foster added a comment - - edited No, this doesn't change the plugin APIs. when I say "User's point of view" I mean the APIs users work with most directly, mainly Velocity.java and VelocityEngine.java. To be clear: methods in Velocity.java and VelocityEngine.java no longer throw java.lang.Exception and java.io.IOException Plugin API is unchanged.
        Hide
        Nathan Bubna added a comment -

        The gump complaints about VelocityTools' WebappUberspect led me here. Looks like the Uberspect plugin API was changed. Should be fixed now...

        Show
        Nathan Bubna added a comment - The gump complaints about VelocityTools' WebappUberspect led me here. Looks like the Uberspect plugin API was changed. Should be fixed now...
        Hide
        Byron Foster added a comment -

        I wonder why Gump has only recently started complaining about it.

        Show
        Byron Foster added a comment - I wonder why Gump has only recently started complaining about it.

          People

          • Assignee:
            Unassigned
            Reporter:
            Byron Foster
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development