[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
RE: [cdt-dev] [DSF] FinalLaunchSequence extensibility
|
> -----Original Message-----
> From: laskava@xxxxxxxxx [mailto:laskava@xxxxxxxxx] On Behalf
> Of Alena Laskavaia
> Sent: Tuesday, July 27, 2010 7:31 PM
> To: Marc Khouzam
> Cc: CDT General developers list.
> Subject: Re: [cdt-dev] [DSF] FinalLaunchSequence extensibility
>
> On Tue, Jul 27, 2010 at 9:48 AM, Marc Khouzam
> <marc.khouzam@xxxxxxxxxxxx> wrote:
> >> -----Original Message-----
> >> From: cdt-dev-bounces@xxxxxxxxxxx
> >> [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of Alena Laskavaia
> >> Sent: Monday, July 26, 2010 11:48 PM
> >> To: CDT General developers list.
> >> Subject: Re: [cdt-dev] [DSF] FinalLaunchSequence extensibility
> >>
> >> I have another idea.
> >> How about make FinalLaunchSequence an API where all steps "execute"
> >> methods defined as protected methods with certain prefix.
> >> The steps initialization function would form steps array
> by taking all
> >> method of a current FinalLaunchSequence class (which could be
> >> overriden)
> >> and instantiate a Step classes with a method name as a parameter.
> >> Order would be determined by method name. Execute function
> would call
> >> a method using reflection.
> >
> > This seems to be along the same lines as the previous suggestions.
> > But instead of overriding the entire step we would override the
> > execute method only.
> > I think overriding the entire step is more flexible because some
> > steps actually define subclasses or members.
>
> And what is the relevance of defining members and subclasses in Step
> method itself?
> You can define it in any other class:
I'm guessing you changed your mind and wanted to respond to this point
as you did below.
> >> a method using reflection.
> >
> > This seems to be along the same lines as the previous suggestions.
> > But instead of overriding the entire step we would override the
> > execute method only.
> > I think overriding the entire step is more flexible because some
> > steps actually define subclasses or members.
> I don't see how does it matter? If only execute method is used having
> "members" in Step class does not
> add benefits. You can always create another class to put this stuff it
> does not have to be in "Step", i.e.
> class MyClass {
> your memeber;
> void execute() {...}
> }
> protected void step_03_1_establishConnection(){
> new MyClass().execute();
> }
If you look at the step in FLS which specifies a core file, you can
see that there is a subclass defined right in the extension of the Step.
It just keeps that code localized to where it is needed.
> > If I understand correctly, the added value of this suggestion
> > is the automatic sorting by method name.
> > As Mikhail pointed out, re-ordering is then a problem.
> Not really. After sequence is created it can be re-arranged
> in array itself.
But then what is the point of having indexes in the names?
> Also added value is a big decrease in a code size you have to write
> which significantly increase readability and maintainability
Allowing to properly extend FLS would give this better
readability and maintainability, however, I'm trying to figure out
the difference/value between your solution and the other ones that
have been proposed.
> > For ordering I would hard-code a number in the name, but
> > I would create an array that lists all the methods
> > in the right order. This can be overridden more easily.
> > But is still not very future-proof for extending classes.
>
> That is what initialize method is doing, see below
Sorry, my sentence should have read "I would _not_ hard-code
a number in the name". What is the value of that number, if
the extending class can modify the array anyway?
> >> Client would override this and
> >> 1) to change step - override it
> >> 2) to remove step - override and make empty
> >> 3) to add step (for example between 3 and 4) add a method
> >>
> >> protected void step_03_1_establishConnection(){...}
> >>
> >>
> >> Old cut & paste method would work too..
> >>
> >>
> >> Now comments regarding "we never will able be fix it again":
> >> a) For cdt implementation we can make another class that
> extends FLS
> >> which is not an API - so we can do whatever we want there
> >
> > But then base FLS will no longer be updated, and changes
> (some bug fixes)
> > will not be propagated to classes that extend it. This
> comes down to
> > having people copy/paste the existing FLS.
>
> Depends. We only have problem with maintenance build, this is a
> solution.
Why do we only have a problem for the maintenance build?
> Solution for head should be always changing parent class.
> Do we have a bug for that? I post a patch and we can discuss it better
> when it is all written.
Thanks.
Let's continue this in the bug.
> >> b) We can add methods/steps in minor revision change releases
> >> c) In maintenance release if we need to add a step (hell
> of bug fix)
> >> we can always add private method, and make it protected later
> >
> > Then the fix won't be propagated to extending classes.
> What do you mean? I can test if reflection can read private method, I
> am sure it can.
> You want be able to call super for that, but again adding step is
> probably should not be done in maint. release anyway.
Right. I was confused.
> >
> >> step class for this would look like
> >> FStep extends Step{
> >> FStep(String executeMethod) {
> >> this.method = excuteMethod;
> >> }
> >> public void execute(RequestMonitor requestMonitor) {
> >> call_method(method, requestMonitor);
> >> }
> >> }
> >>
> >> and
> >> void protected initializeSteps
> >> {
> >> String[] method = getSortedMethods();
> >> fSteps = new Steps[method.length];
> >> for (i=0;i<method.length;i++) {
> >> fSteps[i]=new FStep(method);
> >> }
> >> }
> >>
> >> On Fri, Jul 9, 2010 at 8:51 AM, Vladimir Prus
> >> <vladimir@xxxxxxxxxxxxxxxx> wrote:
> >> > On Thursday 08 July 2010 23:35:04 Mikhail Khodjaiants wrote:
> >> >
> >> >> On 08/07/2010 2:54 PM, Marc Khouzam wrote:
> >> >> >> -----Original Message-----
> >> >> >> From: cdt-dev-bounces@xxxxxxxxxxx
> >> >> >> [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of
> >> Mikhail Khodjaiants
> >> >> >> Sent: Thursday, July 08, 2010 2:27 PM
> >> >> >> To: cdt-dev@xxxxxxxxxxx
> >> >> >> Subject: Re: [cdt-dev] [DSF] FinalLaunchSequence
> extensibility
> >> >> >>
> >> >> >> On 08/07/2010 2:04 PM, Marc Khouzam wrote:
> >> >> >>
> >> >> >>> That is interesting. It's like the subSequence
> solution, with
> >> >> >>> each subSequence being a single step. How would
> you define the
> >> >> >>> step ids?
> >> >> >>>
> >> >> >>>
> >> >> >> Yes, that's correct. See the Andy Jin's comment
> >> regarding granularity.
> >> >> >> I was thinking of the traditional Eclipse style strings:
> >> >> >> <plugin_id>.steps.gdbinit, for instance.
> >> >> >>
> >> >> > Thinking about it some more, isn't this very much like
> >> Vladimir's early
> >> >> > suggestion:
> >> >> >
> >> >> >> - Add protected members for each step of
> >> FinalLaunchSequence, e.g.:
> >> >> >>
> >> >> >> protected Step getTrackerStep = new Step() { .... } ;
> >> >> >>
> >> >> >> - Add factory methods, e.g.:
> >> >> >>
> >> >> >> protected Step getTrackerStep() { .... }
> >> >> >>
> >> >> It is similar. But the difference is to have a steps
> >> library outside of
> >> >> FinalLaunchSequence.
> >> >
> >> > For the record, I don't care much whether the standard
> >> steps are defined as protected
> >> > methods inside FinalLaunchSequence or as standalone
> >> classes, or as factory
> >> > functions. If the name of step does not include the index
> >> of the step, then
> >> > all those solutions have pretty much the same properties:
> >> >
> >> > - A predefined step may not be removed without breaking API
> >> > - An order change in FinalLaunchSequence will not be
> automatically
> >> > picked by derived classes.
> >> >
> >> > I, personally, can live with both those restrictions -- as
> >> soon as my launch sequence
> >> > can cleanly reuse standard steps.
> >> >
> >> > I think that using string identifiers as indices into some
> >> hash, as opposed as Java-level
> >> > compile-time identifiers is much more brittle, though.
> >> >
> >> >
> >> > Thanks,
> >> >
> >> > --
> >> > Vladimir Prus
> >> > CodeSourcery
> >> > vladimir@xxxxxxxxxxxxxxxx
> >> > (650) 331-3385 x722
> >> > _______________________________________________
> >> > cdt-dev mailing list
> >> > cdt-dev@xxxxxxxxxxx
> >> > https://dev.eclipse.org/mailman/listinfo/cdt-dev
> >> >
> >> _______________________________________________
> >> cdt-dev mailing list
> >> cdt-dev@xxxxxxxxxxx
> >> https://dev.eclipse.org/mailman/listinfo/cdt-dev
> >>
>
