[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [cdt-dev] [DSF] FinalLaunchSequence extensibility
|
Replied on bug
https://bugs.eclipse.org/bugs/show_bug.cgi?id=321084
On Thu, Jul 29, 2010 at 9:29 AM, Marc Khouzam <marc.khouzam@xxxxxxxxxxxx> wrote:
>> -----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
>> >>
>>
