[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] [DSF] FinalLaunchSequence extensibility

https://bugs.eclipse.org/bugs/show_bug.cgi?id=321084


On Tue, Jul 27, 2010 at 7:31 PM, Alena Laskavaia
<elaskavaia.cdt@xxxxxxxxx> wrote:
> 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
>>>
>