Velocity
  1. Velocity
  2. VELOCITY-219

Velocity list operator should return a Java List reference rather than ArrayList reference

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Invalid
    • Affects Version/s: 1.4
    • Fix Version/s: 1.5
    • Component/s: Engine
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: All

      Description

      In the 1.x releases of Velocity, the list operator [] returns an ArrayList
      reference instead of a List reference. In the future (2.x), a cleaner API would
      be to return a List reference.

        Activity

        Hide
        Henning Schmiedehausen added a comment -

        Close all resolved issues for Engine 1.5 release.

        Show
        Henning Schmiedehausen added a comment - Close all resolved issues for Engine 1.5 release.
        Hide
        Daniel Rall added a comment -

        In the previous comment, Henning must've been referring to the commit he made in r471881. While the changes from that commit were all good improvements, they were also all internal. While it's been so long since I filed this remdiner issue for myself that I don't recall exactly what prompted it, I am sure that it was caused by the presence of ArrayList in some public API.

        Show
        Daniel Rall added a comment - In the previous comment, Henning must've been referring to the commit he made in r471881. While the changes from that commit were all good improvements, they were also all internal. While it's been so long since I filed this remdiner issue for myself that I don't recall exactly what prompted it, I am sure that it was caused by the presence of ArrayList in some public API.
        Hide
        Henning Schmiedehausen added a comment -

        Ok, after some debugging and code browsing, I'm a bit confused.

        The ArrayList object comes out of ASTObjectArray#value() (for the normal array case) or ASTIntegerRange#value() for the range ([1..10]) case. They both create an ArrayList object. However, the actual API is Object value(InternalContextAdapter context), so there is no guarantee that this will be any assumed type anyway.

        We do have not a single method (at least none that I could find) that returns an explicit ArrayList anywhere. We have a very very small number of explicit ArrayList variable declarations that I will change to List. But aside from that I see nothing that we could/must change for API 2.0.

        If you have more information, please reopen that issue. Thanks.

        Show
        Henning Schmiedehausen added a comment - Ok, after some debugging and code browsing, I'm a bit confused. The ArrayList object comes out of ASTObjectArray#value() (for the normal array case) or ASTIntegerRange#value() for the range ( [1..10] ) case. They both create an ArrayList object. However, the actual API is Object value(InternalContextAdapter context), so there is no guarantee that this will be any assumed type anyway. We do have not a single method (at least none that I could find) that returns an explicit ArrayList anywhere. We have a very very small number of explicit ArrayList variable declarations that I will change to List. But aside from that I see nothing that we could/must change for API 2.0. If you have more information, please reopen that issue. Thanks.
        Hide
        Henning Schmiedehausen added a comment -

        Well it does return a List because ArrayList implements List. Or did I get this wrong. What method / code do you specifically refer to? It might make sense to fix that for 1.5 because if anyone really relies on the operator returning an ArrayList, they're screwed anyway.

        Show
        Henning Schmiedehausen added a comment - Well it does return a List because ArrayList implements List. Or did I get this wrong. What method / code do you specifically refer to? It might make sense to fix that for 1.5 because if anyone really relies on the operator returning an ArrayList, they're screwed anyway.
        Hide
        Will Glass-Husain added a comment -

        Makes sense to me.

        Show
        Will Glass-Husain added a comment - Makes sense to me.

          People

          • Assignee:
            Unassigned
            Reporter:
            Daniel Rall
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development