Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Date: Sun, 17 Mar 2013 10:24:13 +0000 (UTC)
From: "Thomas Neidhart (JIRA)" <jira@apache.org>
To: issues@commons.apache.org
Message-ID: <JIRA.12412112.1231595908897.453660.1363515853293@arcas>
In-Reply-To: <JIRA.12412112.1231595908897@arcas>
References: <JIRA.12412112.1231595908897@arcas>
Subject: [jira] [Commented] (COLLECTIONS-310) Modifications of a
 SetUniqueList.subList() invalidate the parent list
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Thomas Neidhart commented on COLLECTIONS-310:
---------------------------------------------

Hi Thomas,

I have looked through both variants, but am still undecided.
Generally, I think we should keep the inheritance to AbstractSerializableListDecorator, as the class should remain a decorator. Adding more fields to the actual class also has the down-side that we need to make sure that serialization still works correctly, so I would prefer to create an inner static View class, which is returned when calling subList.

I will further play with your patch, and give you more feedback.

Thanks!
                
> 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