Commons Email
  1. Commons Email
  2. EMAIL-24

[email][PATCH] Refactor Hashtable usage to Map

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Operating System: other
      Platform: Other

      Description

      This patch refactors the usage of Hashtable to the Map interface. Coding to
      interfaces as opposed to implementations is considered a best practice and this
      will also remove two errors from the Checkstyle report.

      Thanks,
      Eric Spiegelberg

        Activity

        Hide
        Eric Spiegelberg added a comment -

        Created an attachment (id=15031)
        Refactors Hashtable usage to Map

        I agree with you that breaking existing API's is a bad thing. However, the
        project is currently at 1.0-dev and the main page itself states:

        • The code is unreleased
        • Methods and classes can and will appear and disappear without warning

        While changing the API at this point is less than ideal, most people would
        accept that there exists a potential of change between 1.0-dev and 1.0. When it
        comes down to it, all we're talking about is changing a public method's return
        object from a Hashtable to a Map. This is likely the last opportunity to make
        this (trivial) change before the v1.0 release and it would then have to be
        postponed until 1.1 (or whatever). I think changing it now is in everyone's
        best interest.

        Thanks,
        Eric

        Eric Spiegelberg wrote:

        > Matt Benson wrote:
        >
        >>I just joined the list myself... (for sandbox
        >>commons-pgp) but I noticed that in the non-checkstyle
        >>changes from this patch, the signature of a public
        >>method is modified in at least one place. This will
        >>break already-compiled code running against the
        >>library. Over in Ant-land we consider that bad; I can
        >>only assume the same would be true in Jakarta commons.
        >>
        >>$0.02,
        >>Matt

        Show
        Eric Spiegelberg added a comment - Created an attachment (id=15031) Refactors Hashtable usage to Map I agree with you that breaking existing API's is a bad thing. However, the project is currently at 1.0-dev and the main page itself states: The code is unreleased Methods and classes can and will appear and disappear without warning While changing the API at this point is less than ideal, most people would accept that there exists a potential of change between 1.0-dev and 1.0. When it comes down to it, all we're talking about is changing a public method's return object from a Hashtable to a Map. This is likely the last opportunity to make this (trivial) change before the v1.0 release and it would then have to be postponed until 1.1 (or whatever). I think changing it now is in everyone's best interest. Thanks, Eric Eric Spiegelberg wrote: > Matt Benson wrote: > >>I just joined the list myself... (for sandbox >>commons-pgp) but I noticed that in the non-checkstyle >>changes from this patch, the signature of a public >>method is modified in at least one place. This will >>break already-compiled code running against the >>library. Over in Ant-land we consider that bad; I can >>only assume the same would be true in Jakarta commons. >> >>$0.02, >>Matt
        Hide
        Simon Kitching added a comment -

        Is Email intended to support v1.1 JVMs? If so, Map is not an option.

        Show
        Simon Kitching added a comment - Is Email intended to support v1.1 JVMs? If so, Map is not an option.
        Hide
        David Graham added a comment -

        Other projects like Validator and BeanUtils have learned the hard way that
        exposing concrete implementation classes in public API is very hard to change
        once released. Hashtable has been effectively deprecated for many years now.
        Converting those references to Map is an easy and obvious change.

        Show
        David Graham added a comment - Other projects like Validator and BeanUtils have learned the hard way that exposing concrete implementation classes in public API is very hard to change once released. Hashtable has been effectively deprecated for many years now. Converting those references to Map is an easy and obvious change.
        Hide
        dion gillard added a comment -

        Committed to SVN

        Show
        dion gillard added a comment - Committed to SVN

          People

          • Assignee:
            Unassigned
            Reporter:
            Eric Spiegelberg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development