You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Sebb (JIRA)" <ji...@apache.org> on 2009/01/26 00:52:59 UTC

[jira] Created: (DBCP-280) SequencedHashMap constructor calls overridable method; throws Error

SequencedHashMap constructor calls overridable method; throws Error
-------------------------------------------------------------------

                 Key: DBCP-280
                 URL: https://issues.apache.org/jira/browse/DBCP-280
             Project: Commons Dbcp
          Issue Type: Bug
            Reporter: Sebb


The constructor SequencedHashMap(Map) calls the public non-final method putAll().

This is likely to cause problems if the class is ever extended. Possible solution: extract the code into a common private method.

The next() method also throws an Error if the return type is unexpected.
It should probably return IllegalStateException or similar.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (DBCP-280) SequencedHashMap constructor calls overridable method; throws Error

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DBCP-280?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673915#action_12673915 ] 

Sebb commented on DBCP-280:
---------------------------

I've had a look at Collections (trunk), and the class SequencedHashMap is deprecated - it recommends using LinkedMap or ListOrderedMap instead  - so the Collections code is unlikely to be fixed.

Unfortunately, LinkedMap is a sub-class of AbstractHashedMap which has the same problem (I'll raise a separate Jira for this).

Perhaps a minimally invasive solution would be to make the SequencedHashMap - or putall() method - final in DBCP?

The former would probably be better, given that the original class is now deprecated.

> SequencedHashMap constructor calls overridable method; throws Error
> -------------------------------------------------------------------
>
>                 Key: DBCP-280
>                 URL: https://issues.apache.org/jira/browse/DBCP-280
>             Project: Commons Dbcp
>          Issue Type: Bug
>            Reporter: Sebb
>
> The constructor SequencedHashMap(Map) calls the public non-final method putAll().
> This is likely to cause problems if the class is ever extended. Possible solution: extract the code into a common private method.
> The next() method also throws an Error if the return type is unexpected.
> It should probably return IllegalStateException or similar.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (DBCP-280) SequencedHashMap constructor calls overridable method; throws Error

Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DBCP-280?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Phil Steitz resolved DBCP-280.
------------------------------

    Resolution: Won't Fix

This class was copied from Commons Collections 2.1 and has package scope.  

> SequencedHashMap constructor calls overridable method; throws Error
> -------------------------------------------------------------------
>
>                 Key: DBCP-280
>                 URL: https://issues.apache.org/jira/browse/DBCP-280
>             Project: Commons Dbcp
>          Issue Type: Bug
>            Reporter: Sebb
>
> The constructor SequencedHashMap(Map) calls the public non-final method putAll().
> This is likely to cause problems if the class is ever extended. Possible solution: extract the code into a common private method.
> The next() method also throws an Error if the return type is unexpected.
> It should probably return IllegalStateException or similar.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (DBCP-280) SequencedHashMap constructor calls overridable method; throws Error

Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DBCP-280?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673904#action_12673904 ] 

Phil Steitz commented on DBCP-280:
----------------------------------

I would like to leave the collections classes included in dbcp in exactly the same state that they were when incorporated unless and until a dbcp-impacting bug with one of these classes is identified.  Both SequencedHashMap and LRUMap (the class actually used) have package scope and javadoc headers indicating their lineage and that they are not to be made public.  I do not want us to take on maintaining forked versions unless and until we need to.

> SequencedHashMap constructor calls overridable method; throws Error
> -------------------------------------------------------------------
>
>                 Key: DBCP-280
>                 URL: https://issues.apache.org/jira/browse/DBCP-280
>             Project: Commons Dbcp
>          Issue Type: Bug
>            Reporter: Sebb
>
> The constructor SequencedHashMap(Map) calls the public non-final method putAll().
> This is likely to cause problems if the class is ever extended. Possible solution: extract the code into a common private method.
> The next() method also throws an Error if the return type is unexpected.
> It should probably return IllegalStateException or similar.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Reopened: (DBCP-280) SequencedHashMap constructor calls overridable method; throws Error

Posted by "Sebb (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DBCP-280?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sebb reopened DBCP-280:
-----------------------


I don't see how the scope or the origin of the code affects this.

If the class will never be extended, then make the class final.
If the method will never be overridden, then make the method final.
Otherwise, extract the code into a private method as originally suggested.

Since the fix to make the code safe is easy to do, it seems to me that it is worth doing to avoid potential problems later.
I.e. defensive programming.

> SequencedHashMap constructor calls overridable method; throws Error
> -------------------------------------------------------------------
>
>                 Key: DBCP-280
>                 URL: https://issues.apache.org/jira/browse/DBCP-280
>             Project: Commons Dbcp
>          Issue Type: Bug
>            Reporter: Sebb
>
> The constructor SequencedHashMap(Map) calls the public non-final method putAll().
> This is likely to cause problems if the class is ever extended. Possible solution: extract the code into a common private method.
> The next() method also throws an Error if the return type is unexpected.
> It should probably return IllegalStateException or similar.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (DBCP-280) SequencedHashMap constructor calls overridable method; throws Error

Posted by "Mark Thomas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DBCP-280?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mark Thomas resolved DBCP-280.
------------------------------

    Resolution: Won't Fix

SequencedHashMap cannot be made final. It is extended by LRUMap.

Yes, the code could be improved. However code correctness is not a sufficient justification to fork the code when:
- there is no real world bug reported as a result of these issues
- the classes are clearly marked as to lineage and that they are not to be made public

> SequencedHashMap constructor calls overridable method; throws Error
> -------------------------------------------------------------------
>
>                 Key: DBCP-280
>                 URL: https://issues.apache.org/jira/browse/DBCP-280
>             Project: Commons Dbcp
>          Issue Type: Bug
>            Reporter: Sebb
>
> The constructor SequencedHashMap(Map) calls the public non-final method putAll().
> This is likely to cause problems if the class is ever extended. Possible solution: extract the code into a common private method.
> The next() method also throws an Error if the return type is unexpected.
> It should probably return IllegalStateException or similar.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (DBCP-280) SequencedHashMap constructor calls overridable method; throws Error

Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DBCP-280?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Phil Steitz closed DBCP-280.
----------------------------


> SequencedHashMap constructor calls overridable method; throws Error
> -------------------------------------------------------------------
>
>                 Key: DBCP-280
>                 URL: https://issues.apache.org/jira/browse/DBCP-280
>             Project: Commons Dbcp
>          Issue Type: Bug
>            Reporter: Sebb
>
> The constructor SequencedHashMap(Map) calls the public non-final method putAll().
> This is likely to cause problems if the class is ever extended. Possible solution: extract the code into a common private method.
> The next() method also throws an Error if the return type is unexpected.
> It should probably return IllegalStateException or similar.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.