[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
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:

>> 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 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.
Also added value is a big decrease in a code size you have to write
which significantly increase readability and maintainability

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

>
>> 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. 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.

>
>> 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.

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