Skip to main content

Please review fix for [Issue 147]

3 replies [Last post]
Anonymous

Hi Vladimir,

please review fix for [Issue 147]: "Interactive agent is missed while starting
tests through 'Run' button".https://cqme.dev.java.net/issues/show_bug.cgi?id=147
The changeset: http://fisheye4.cenqua.com/changelog/cqme/?cs=916

Thanks,
Alexander

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
Vladimir Sizikov

Hi Alexander,

Another way to fix that would be just to replace value by this.value:

interactiveAgent = ( this.value == YES );

Question's setValue() method "canonicalize" the value. :)

Still, using equals is just much more safer.

So, I have no problems with the proposed fix for this particular
problem. But I have more general problem with setting the
'interactiveAgent' in setValue().

For example, if this question is going to be hidden for some reason,
or out of the interview main path, the value of 'interactiveAgent'
will be out of sync and we've seen the tricky interview bugs caused by
this.

Generally, I prefer to have a method isInteractiveAgent() that deals
with paths and question values, and this method is _always_ is
up-to-date, and if the question is not on the path for some reason, we
handle that too, and then we can invoke this method any time we want
with no need to write any extra conditions and checks.

Take a look, for example, at VmInterview.isPreverifierMode():

boolean isPreverifierMode() {
if (pathContains(qProduct)) {
return PREVERIFIER.equals(qProduct.getValue());
} else {
return false;
}
}

I think this is a good pattern to follow.
What do you think?

Thanks,
--Vladimir

On Thu, May 17, 2007 at 12:52:24PM +0400, Alexander Alexeev wrote:
> Hi Vladimir,
>
> please review fix for [Issue 147]: "Interactive agent is missed while
> starting
> tests through 'Run'
> button".https://cqme.dev.java.net/issues/show_bug.cgi?id=147
> The changeset: http://fisheye4.cenqua.com/changelog/cqme/?cs=916
>
> Thanks,
> Alexander

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net

Alexander Alexeev

Vladimir,

agree with you. The method isInteractiveAgent() is much more safe. Please review
the fix: http://fisheye4.cenqua.com/changelog/cqme/?cs=919

Thanks,
Alexander

Vladimir Sizikov wrote:
> Hi Alexander,
>
> Another way to fix that would be just to replace value by this.value:
>
> interactiveAgent = ( this.value == YES );
>
> Question's setValue() method "canonicalize" the value. :)
>
> Still, using equals is just much more safer.
>
> So, I have no problems with the proposed fix for this particular
> problem. But I have more general problem with setting the
> 'interactiveAgent' in setValue().
>
> For example, if this question is going to be hidden for some reason,
> or out of the interview main path, the value of 'interactiveAgent'
> will be out of sync and we've seen the tricky interview bugs caused by
> this.
>
> Generally, I prefer to have a method isInteractiveAgent() that deals
> with paths and question values, and this method is _always_ is
> up-to-date, and if the question is not on the path for some reason, we
> handle that too, and then we can invoke this method any time we want
> with no need to write any extra conditions and checks.
>
> Take a look, for example, at VmInterview.isPreverifierMode():
>
> boolean isPreverifierMode() {
> if (pathContains(qProduct)) {
> return PREVERIFIER.equals(qProduct.getValue());
> } else {
> return false;
> }
> }
>
> I think this is a good pattern to follow.
> What do you think?
>
> Thanks,
> --Vladimir
>
> On Thu, May 17, 2007 at 12:52:24PM +0400, Alexander Alexeev wrote:
>> Hi Vladimir,
>>
>> please review fix for [Issue 147]: "Interactive agent is missed while
>> starting
>> tests through 'Run'
>> button".https://cqme.dev.java.net/issues/show_bug.cgi?id=147
>> The changeset: http://fisheye4.cenqua.com/changelog/cqme/?cs=916
>>
>> Thanks,
>> Alexander
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
> For additional commands, e-mail: meframework-help@cqme.dev.java.net
>

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net

Vladimir Sizikov

Alexander,

Looks good now. :)

Thanks,
--Vladimir

On Thu, May 17, 2007 at 02:07:42PM +0400, Alexander Alexeev wrote:
> Vladimir,
>
> agree with you. The method isInteractiveAgent() is much more safe. Please
> review the fix: http://fisheye4.cenqua.com/changelog/cqme/?cs=919
>
> Thanks,
> Alexander
>
> Vladimir Sizikov wrote:
> >Hi Alexander,
> >
> >Another way to fix that would be just to replace value by this.value:
> >
> >interactiveAgent = ( this.value == YES );
> >
> >Question's setValue() method "canonicalize" the value. :)
> >
> >Still, using equals is just much more safer.
> >
> >So, I have no problems with the proposed fix for this particular
> >problem. But I have more general problem with setting the
> >'interactiveAgent' in setValue().
> >
> >For example, if this question is going to be hidden for some reason,
> >or out of the interview main path, the value of 'interactiveAgent'
> >will be out of sync and we've seen the tricky interview bugs caused by
> >this.
> >
> >Generally, I prefer to have a method isInteractiveAgent() that deals
> >with paths and question values, and this method is _always_ is
> >up-to-date, and if the question is not on the path for some reason, we
> >handle that too, and then we can invoke this method any time we want
> >with no need to write any extra conditions and checks.
> >
> >Take a look, for example, at VmInterview.isPreverifierMode():
> >
> >boolean isPreverifierMode() {
> > if (pathContains(qProduct)) {
> > return PREVERIFIER.equals(qProduct.getValue());
> > } else {
> > return false;
> > }
> >}
> >
> >I think this is a good pattern to follow.
> >What do you think?
> >
> >Thanks,
> > --Vladimir
> >
> >On Thu, May 17, 2007 at 12:52:24PM +0400, Alexander Alexeev wrote:
> >>Hi Vladimir,
> >>
> >>please review fix for [Issue 147]: "Interactive agent is missed while
> >>starting
> >>tests through 'Run'
> >>button".https://cqme.dev.java.net/issues/show_bug.cgi?id=147
> >>The changeset: http://fisheye4.cenqua.com/changelog/cqme/?cs=916
> >>
> >>Thanks,
> >>Alexander
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
> >For additional commands, e-mail: meframework-help@cqme.dev.java.net
> >
>

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net