This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
First apologies if this was already discussed but I did not find it on projects-dev. The FileSet class contains two methods that can mutate its content: boolean clearBrokenLinks(); iterator().remove(); - according to javadoc the implementation is optional here I'm wondering whether it wouldn't be better to make the FileSet immutable through the API i.e. the mutations could only occur internally by the FileSet implementation itself. It would mean that the iterator() would never implement remove() and clearBrokenLinks() would return a filtering FileSet instance instead of boolean. That instance would == this in case there were no broken links otherwise it would be a different FileSet instance with removed broken links. IMHO having the FileSet "externally" immutable could allow for certain constructions e.g.: public class A { protected void b(FileSet fs) { // subclasses may modify content // by e.g. clearBrokenLinks() } void a() { FileSet fs = ... b(fs); // Now if a() would wish to use fs (with original content) // it should clone it before passing to b() // because of unpredictable implementation of b() // the fs can now contain a different content. // If FileSet would be immutable this could not happen // (only internal modifications could be done but that's fine). } } As FileSets are expected to contain large sets of data this change could lead to easier sharing in certain situations allowing to possibly save more resources. Finally although it's just a minor argument there is a MutableFileSet class which gives a notion that the FileSet class should not be mutated by the APIs.
#1 clearBrokenLinks - IMO this method should be moved to MutableFileSet; both FileSets.createFileObjectSet and FileSets.createDataObjectSet return MutableFileSet and allow to specify default behavior (keepBrokenLinks param). Moreover this param should also be supported by createURLFileSet I think. #2 iterator - the problem here is that FileSet.iterator is overriden by subclasses which may or may not be mutable. Your suggestion will lead to having another getMutableIterator method for mutable FileSet subclasses. IMO we could slightly improve the javadoc saying more explicitly that generic FileSet isn't mutable (and the Iterator.remove doesn't work), but some subclasses may support this (e.g. MutableFileSet).
To Mila: if the fileset would be immutable how clients would affect the fileset's persistent state? We would have to entirely redesign the way how filesets are persisted now. FileSet FileSets.immutable(FileSet) seems to be what you ask IMO. To Vita: #1 a) createURLFileSet does not make sense in connection to broken links. How will you identify broken links when you know nothing about what it is behind them? b) in regard to move clearBrokenLinks to MutableFileSet it is question it is worth to do that. Definitely we could do a generalization to give the method back if needed in future. #2 I would not be such strict to prevent the Iterator.remove implementation in general. You are right about the documentation. IMO each instance produced via factories FileSets.createXXX has to be extended in order to document this behavior.
#1a. Partially true, but I meant the situation when resolver isn't able to decode the URL from internal format (e.g. because of missing reference point in case of project's ReferencesPointsResolver).
You are right. I omitted that case.
I'm not sure I have described it precisely. I'm not asking for making all of the FileSet plus all of its extensions externally immutable. I'm asking for making only org.netbeans.api.filesets.FileSet abstract class immutable by removing the part of its API access that allows the mutation (and in case of the iterator().remove() explicitly stating that the it must not be called by the clients). It would imply that if a method is given a FileSet parameter it is not allowed to mutate its content because there would be no mutation methods except iterator().remove() which is explicitly stated not to be used. On the other hand if a method is given a MutableFileSet parameter it's allowed to freely mutate its content by the API methods available in MutableFileSet. Having MutableFileSet and other mutable extensions of FileSet is fine and I'm definitely not against it. Even a method getting FileSet could still try to cast it to MutableFileSet and mutate it but it's clearly a bad thing that's not supported. In fact we could even leave it as it is now and state explicitly that a particular fileset parameter in a given method should not be mutated by the method's implementation (in the javadoc for the particular method) but IMHO it would be much better if the FileSets API could enforce this directly.
I think we understand your concern. Let me summarize it: 1. createURLFileSet should take keepBrokenLinks prameter 2. clearBrokenLinks method should be moved in MutableFileSet (moving it up in the inheritance hierarchy later is compatible change) 3. the javadoc of FileSet.iterator should be updated to say that the iterator doesn't support removing in general, but there could be FileSet implementations which supports it Is this correct, acceptable?
Yes, that's fine, thanks.
As described in http://www.netbeans.org/servlets/ReadMsg?msgId=619519&listName=nbdiscuss the current work on projects prototype has been stopped.
--> VERIFIED