[jdom-interest] Bug in PartialList add(index, object).

[Ken Rune Helland]

I saw partiallist was to be rewritten but still I'll report this
in case the person implementing the new partiallist is not aware
of this

 From beta6 PartialList:

public void add(int index, Object current) {
         if (index == size()) {
             // Adding to "length" spot is special case meaning to our end
             addLast(current);
         }
         else {
             backingList.add(backingList.indexOf(get(index)), current);

             if (current instanceof Element) {
                 ((Element)current).setParent(parent);  // null is OK
             }
             super.add(index, current);
         }
     }

The problem is the adding to the backingList.
If the PartialList is obtained by Element.getMixedContent()
it may contain String objects so the get(index) may return a String.

This is a problem because String.equals(Object obj) is not defined as
this == obj (and since strings a immutable and are reused it might be
the same object anyway), but check if the internal char arrays contains the 
same characters.

So if i want to insert an objetc in a position that happens to
be occupied by a String and there is an equal string earlier in the
list the objetc will be inserted at the index of the first matching
String and not the index i wanted.

This might be an argument to create a Text class

A fix is:

public void add(int index, Object current) {
   if (index == size()) {
    // Adding to "length" spot is special case meaning to our end
    addLast(current);
   }
   else {
     Object o = get(index);

     if( o instanceof String )
     {
       // asumes that if the PartialList contains String objects
       // it is actually equal to the backingList
       backingList.add(index,current);
     }
     else
     {
       backingList.add(backingList.indexOf(o), current);
     }

     if (current instanceof Element) {
          ((Element)current).setParent(parent);  // null is OK
     }

     super.add(index, current);
   }
}


This asumes the PartilalList only contains Strings if it
is created by Element.getMixedContent() and is equal to the
backingList.


a diff created by C:\>diff -b -B -l oldfile newfile:

2001-03-08 15:09 *cs\cvs_dirs\source\org\jdom\PartialList.java Page    1


325c325,337
<             backingList.add(backingList.indexOf(get(index)), current);
---
 >                                               Object o = get(index);
 >
 >                                               if( o instanceof String )
 >                                               {
 >                                                       // asumes that if 
the PartialList contains String objects
 >                                                       // it is actually 
equal to the backingList
 >
 > 
backingList.add(index,current);
 >                                               }
 >                                               else
 >                                               {
 > 
backingList.add(backingList.indexOf(o), current);
 >                                               }
?


Maybe the FAQ shoud have an entry about creating patches?
what diff options are prefered to use?

Maybe there are other PartialList metods that have this problem
but as someone is already reimplementing it I havent cared to look.


KenR