[jira] [Comment Edited] (COLLECTIONS-310) Modifications of a SetUniqueList.subList() invalidate the parent list

["Thomas Vahrst (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/COLLECTIONS-310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13642987#comment-13642987 ] 

Thomas Vahrst edited comment on COLLECTIONS-310 at 4/26/13 4:44 PM:
--------------------------------------------------------------------

Hi Thomas,
Christian Semrau, who reported this issue, provides in the issue description two possible solutions to the inconsistency of sublist. In a nutshell:
When adding a value to a SubList and this value exists in the underlying SetUniqueList,
1. the subList behaves like a SetUniqueList and the existing value is *moved* to the new position - thus breaking the contract of the underlying SetUniqueList.
2. nothing happens because the value already exists - which means that the *sublist* does not behave like a SetUniqueList.

Christian preferred the first solution, so I implemented all JUnit tests (and the proposed changes of SetUniqueList) according to solution no. 1


If we decide to switch to solution no. 2, I would at first change all unit tests according to this solution. Ok?

btw: invoking set() method with a value that exists outside the sublist will move the value to the new position, right? And also: removing a value in a sublist which exists outside the sublist range will remove this value in the underlying SEtUnqiqueList?

(btw 2: did I allready mention that I am not happy at all with our attempt to solve a obviously inconsistent construct? Whether solution No. 1 or 2: I wonder how to write a good javadoc comment for sublist() which describes the behavior in a clear manner... The possibly most simple javadoc could read: "This method returns a sublist which is umodifiable." ;-)

                
      was (Author: t.vahrst):
    Hi Thomas,
Christian Semrau, who reported this issue, provides in the issue description two possible solutions to the inconsistency of sublist. In a nutshell:
When adding a value to a SubList and this value exists in the underlying SetUniqueList,
1. the subList behaves like a SetUniqueList and the existing value is *moved* to the new position - thus breaking the contract of the underlying SetUniqueList.
2. nothing happens because the value already exists - which means that the *sublist* does not behave like a SetUniqueList.

Christian preferred the first solution, so I implemented all JUnit tests (and the proposed changes of SetUniqueList) according to solution no. 1


If we decide to switch to solution no. 2, I would at first change all unit tests according to this solution. Ok?

btw: invoking set() method with a value that exists outside the sublist will move the value to the new position, right?

(btw 2: did I allready mention that I am not happy at all with our attempt to solve a obviously inconsistent construct? Whether solution No. 1 or 2: I wonder how to write a good javadoc comment for sublist() which describes the behavior in a clear manner... The possibly most simple javadoc could read: "This method returns a sublist which is umodifiable." ;-)

                  
> Modifications of a SetUniqueList.subList() invalidate the parent list
> ---------------------------------------------------------------------
>
>                 Key: COLLECTIONS-310
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-310
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: List
>    Affects Versions: 3.2, Nightly Builds
>            Reporter: Christian Semrau
>            Priority: Minor
>             Fix For: 4.0
>
>         Attachments: SetUniqueList.java, SetUniqueList.patch, SetUniqueListTest.java, SetUniqueListTest.java, SetUniqueList.v2.java
>
>
> The List returned by SetUniqueList.subList() is again a SetUniqueList. The contract for List.subList() says that the returned list supports all the operations of the parent list, and it is backed by the parent list.
> We have a SetUniqueList uniqueList equal to {"Hello", "World"}. We get a subList containing the last element. Now we add the element "Hello", contained in the uniqueList but not in the subList, to the subList.
> What should happen?
> Should the subList behave like a SetUniqueList and add the element - meaning that it changes position in the uniqueList because at the old place it gets removed, so now uniqueList equals {"World", "Hello"} (which fails)?
> Or should the element not be added, because it is already contained in the parent list, thereby violating the SetUniqueList-ness of the subList (which fails)?
> I prefer the former behaviour, because modifications should only be made through the subList and not through the parent list (as explained in List.subList()).
> What should happen if we replace (using set) the subList element "World" with "Hello" instead of adding an element?
> The subList should contain only "Hello", and for the parent list, the old element 0 (now a duplicate of the just set element 1) should be removed (which fails).
> And of course the parent list should know what happens to it (specifically, its uniqueness Set) (which fails in the current snapshot).
> 	public void testSubListAddNew() {
> 		List uniqueList = SetUniqueList.decorate(new ArrayList());
> 		uniqueList.add("Hello");
> 		uniqueList.add("World");
> 		List subList = uniqueList.subList(1, 2);
> 		subList.add("Goodbye");
> 		List expectedSubList = Arrays.asList(new Object[] { "World", "Goodbye" });
> 		List expectedParentList = Arrays.asList(new Object[] { "Hello", "World", "Goodbye" });
> 		assertEquals(expectedSubList, subList);
> 		assertEquals(expectedParentList, uniqueList);
> 		assertTrue(uniqueList.contains("Goodbye")); // fails
> 	}
> 	public void testSubListAddDuplicate() {
> 		List uniqueList = SetUniqueList.decorate(new ArrayList());
> 		uniqueList.add("Hello");
> 		uniqueList.add("World");
> 		List subList = uniqueList.subList(1, 2);
> 		subList.add("Hello");
> 		List expectedSubList = Arrays.asList(new Object[] { "World", "Hello" });
> 		List expectedParentList = Arrays.asList(new Object[] { "World", "Hello" });
> 		assertEquals(expectedSubList, subList);
> 		assertEquals(expectedParentList, uniqueList); // fails
> 	}
> 	public void testSubListSetDuplicate() {
> 		List uniqueList = SetUniqueList.decorate(new ArrayList());
> 		uniqueList.add("Hello");
> 		uniqueList.add("World");
> 		List subList = uniqueList.subList(1, 2);
> 		subList.set(0, "Hello");
> 		List expectedSubList = Arrays.asList(new Object[] { "Hello" });
> 		List expectedParentList = Arrays.asList(new Object[] { "Hello" });
> 		assertEquals(expectedSubList, subList);
> 		assertEquals(expectedParentList, uniqueList); // fails
> 	}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira