Carl,
Giving comments accoring to the index:
2.Changin the name of getChangedResource to getAffectedResources will
just help developers that have worked with IResourceChangeEvent/Delta
and IElementChangeListener more easily use the IModelProviderEvent. But
I agree that the benefin does not outweights the cost of changing such
an API.
1 and 4. I think it is better to have the set* and add* methods in the
implementation of a IModelProviderEvent and keep the constructor of the
implementation with a few params. Something of that sort:
private static class MyModelProviderEvent implements IModelProviderEvent
{
private IModelProvider model;
private IProject proj;
private int event;
private List resources;
public MyModelProviderEvent(int anEventCode,
IModelProvider model, IProject proj) {
setEventCode(anEventCode);
setModel(model);
setProject(proj);
resources = new ArrayList();
}
public void setProject(IProject project) {
proj = project;
}
public void addResource(Object resource) {
resources.add(resource);
}
public void addResources(Collection<Object>
someResources) {
resources.addAll(someResources);
}
public List<Object> getChangedResources() {
return resources;
}
public int getEventCode() {
return event;
}
public IModelProvider getModel() {
return model;
}
public void setChangedResources(List<Object>
newChangedResources) {
throw new UnsupportedOperationException();
}
public void setEventCode(int newEventCode) {
event = newEventCode;
}
public void setModel(IModelProvider newModel) {
model = newModel;
}
public IProject getProject() {
return proj;
}
}
Where the IModelProviderEvent is:
public interface IModelProviderEvent {
public static final int SAVE = 0x1;
public static final int DIRTY = 0x2;
public static final int REMOVED_RESOURCE = 0x4;
public static final int ADDED_RESOURCE = 0x8;
public static final int PRE_DISPOSE = 0x10;
public static final int LOADED_RESOURCE = 0x20;
public static final int UNLOADED_RESOURCE = 0x40;
public static final int KNOWN_RESOURCES_ABOUT_TO_CHANGE = 0x80;
public static final int KNOWN_RESOURCES_CHANGED = 0x100;
public abstract List<Object> getChangedResources();
public abstract IModelProvider getModel();
public abstract IProject getProject();
public abstract int getEventCode();
}
This way when constucting an event the developer can use the set* and
add* methods of the implementation. And not constucting the event with a
single call to the constuctor.
I would prefer methods instead of constructor parameter for the
implementation of IModelProviderEvent. And I like the simplicity of the
constuctor for MyModelProviderEvent- (int anEventCode, IModelProvider
model, IProject proj)
I am currently developing a few more classes that make use of the
IModelProviderEvent - to see what more one might need.
I think that after that I will be able to open meaningful feature
requests.
Best Regards,
Kiril
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]
On Behalf Of Carl Anderson
Sent: Tuesday, 5. February 2008 20:41
To: General discussion of project-wide or architectural issues.
Subject: Re: [wtp-dev] IModelProviderEvent structure, methods and use?
Kiril,
Since I put the ModelProviderListener and ModelEvent stuff
together,
let me try to answer your questions. (And, I am more than happy to work
with you to get these sorts of changes into WTP 3.0. I am moreso
hesitant
about putting these sorts of changes into 2.0.x because of their
nature.)
1. The only reason I can see as to why we shouldn't change the
parameter
to be <? extends Object> instead of <Object> is your point #4. (In
other
words, we should add a parameter to the constructor that is <? extends
Object>, and remove the addResources from the interface.
2. This is just a semantic change. And I don't believe that the
benefit
outweighs the cost of changing such an API. One can easily argue that a
resource has changed if it has been modified, added, or removed from the
model.,
3. As with bug 217418, I agree. When I coded this, I stored the
changed
objects in a List, so I set the return type to be a List. I should have
made it agree with addResources() and made it a Collection.
4. Once again, I agree. I used the set* and add* methods to fill in the
information that I wanted to send, adding methods instead of constructor
parameters, which made it easier to change things around on the class as
I
put it together. I should have switched them all to constructor
parameters
once I had finalized the desired objects for the event. (And you will
note
that Chuck tweaked some of these later from generic calls to specific
calls... but I think we have pretty much finalized the desired
parameters
now.)
Other than item #2, please feel free to open enhancements and submit
patches. (I would prefer it if you deprecate the interface's add* and
set*
methods, giving adopters this release to get off of them.)
FWIW,
- Carl Anderson
WTP programmer
"Mitov, Kiril"
<[EMAIL PROTECTED]>
Sent by:
To
[EMAIL PROTECTED] "General discussion of
project-wide
clipse.org or architectural issues."
<[email protected]>
cc
02/01/2008 08:09
AM
Subject
[wtp-dev] IModelProviderEvent
structure, methods and use?
Please respond to
"General
discussion of
project-wide or
architectural
issues."
<[EMAIL PROTECTED]
org>
Hi,
Thanks for helping me.
I am working on a new ModelProvider and I am currently working on the
notifications.
I have a number of question about IModelProviderEvent and the methods
provided by this class.
I have just opened a enchansment request
https://bugs.eclipse.org/bugs/show_bug.cgi?id=217418
but I then realize that maybe I do not understand the structure of
IModelProviderEvent and its use so here are my questions.
1. Why is the method public abstract void
addResources(Collection<Object>
someResources); with a parameter of Collection<Object>. Cant it be
public
abstract void addResources(Collection<? extends Object> someResources);
This way I will be able to execute
private void processAddedFiles(IModelProviderEvent modelEvent,
Collection<IFile> addedFiles) throws JavaModelException {
modelEvent.addResources(addedFiles);
}
and add the addedFiles collection directly to modelEvent.
2.What is the purpose of public abstract List<Object>
getChangedResources
(); If this method is for retrieving the files that this modelEvent is
desribing wont it be more appropriate to name it getAffectedResources().
Since I can also have removed and added resources. Not just
changedResources.
3.Why should the getChangedResources() method return a List. Since
addResources is accepting Collection won`t it be more appropriate to
return
a Collection. Every implementation may provide a different collection
for
such a general class as IModelProviderEvent
4.What is the purpose of the set* and add* methods. From my point of
view
this methods should be part of the implenetation not from an interface
since the one notified with a model event wont need to add or set a
resource to the event. Only when constucting the event such
functionality
is needed.
I am willing to open enchansment requests and to propose patches
changing
the IModelProviderEvent. I will also change ModelProviderEvent which is
the
default empty implementation, but maybe I am missing something.
Thank you for the help.
Best Regards,
Kiril _______________________________________________
wtp-dev mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/wtp-dev
_______________________________________________
wtp-dev mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/wtp-dev
_______________________________________________
wtp-dev mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/wtp-dev
