Skip to main content

code review for added interview question

1 reply [Last post]
Anonymous

Reply viewing options

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

Hi Alexander,

Transport constants will also be used by CDCDTFInterview clients. We
have the API in CDCDTFInterview:

/**
* Set the transport mode for communication.
*
* @param mode transport mode defined in InterviewConstants
*/
public void setTransportMode(int mode) {
commInterview.setTransport(mode);
}

Thanks,
Allen

Alexander Alexeev wrote:

> Hi Allen,
>
> changes look good. What about my propose?:
> >> Seems that transport constants could be made public straight in the
> >> CommInerview because they are only used for the setting up
> >> CommInetrview and make sense only for it.
>
> Thanks,
> Alexander
>
> Allen Wang wrote:
>
>> Hi Alexander,
>>
>> I agree with your comments. The updated review is attached.
>>
>> Thanks,
>> Allen
>>
>> Alexander Alexeev wrote:
>>
>>> Hi Allen,
>>>
>>> please see the comments below.
>>>
>>> Allen Wang wrote:
>>>
>>>> Hi Alexander,
>>>>
>>>>> Does CommInterview make sense for CDC-stack TCKs without DTF (as
>>>>> for the CLDC-stack)? If yes then it possible to move creation of
>>>>> CommInterview outside DTFInterview in the upper level interview.
>>>>> This allow to reuse DTFInterview in the MIDP mode also. If not
>>>>> then CommInterview in the DTFInterview should be created in CDC
>>>>> mode only.
>>>>>
>>>>>
>>>> I couldn't think of a CDC stack TCK which may need CommInterview
>>>> only. Even if there is one, it can just reuse CommInterview without
>>>> having to touch DTFInterview, right? We can see DTFInterview as a
>>>> wrapper around CommInterview with a few more optional questions
>>>> related to DTF for added convenience for TCKs which have
>>>> distributed tests.
>>>>
>>>> Right now, I don't think MIDP TCK will use DTFInterview because it
>>>> does not care about passive agent. The only benefit for MIDP TCK to
>>>> use DTFInterview is that it could reuse the questions for JavaTest
>>>> host IP address and remote verboseness for free, which is not a big
>>>> deal. Also, DTFInterview exports variables that may have different
>>>> names in MIDP TCK interview, like passiveHost and passivePort. So I
>>>> would say DTFInterview is not ready (or worth) to be shared with
>>>> MIDP TCK interview.
>>>>
>>>> In the spirit of "when in doubt, leave it out", I agree that we
>>>> should just hard code the CDC mode when creating CommInterview and
>>>> remove the argument "mode" from the constructors. We can always add
>>>> support for CLDC stack TCK in DTFInterview if it is required later.
>>>> What do you think?
>>>>
>>>
>>> agreed about hard code CDC mode in DTFInterview and I propose to
>>> rename it to something like CdcDTFInterview.
>>>
>>>>> And I propose to split constants in two classes with appropriate
>>>>> names: one class for transports' constants and another for modes'
>>>>> constants.
>>>>>
>>>>>
>>>> I don't see any significant benfit for doing this. Why do we need
>>>> to introduce an extra class for only two constants?
>>>>
>>>
>>> Currently absolutely different entities combined into one class with
>>> meaningless name. It's really hard for the third person to
>>> understand what for these constants are.
>>>
>>> Seems that transport constants could be made public straight in the
>>> CommInerview because they are only used for the setting up
>>> CommInetrview and make sense only for it.
>>>
>>> Thanks,
>>> Alexander
>>>
>>>> Thanks,
>>>> Allen
>>>>
>>>> meframework@mobileandembedded.org wrote:
>>>>
>>>>> Hi Allen,
>>>>>
>>>>> OK, lets don't touch VmInterview.
>>>>>
>>>>> Does CommInterview make sense for CDC-stack TCKs without DTF (as
>>>>> for the CLDC-stack)? If yes then it possible to move creation of
>>>>> CommInterview outside DTFInterview in the upper level interview.
>>>>> This allow to reuse DTFInterview in the MIDP mode also. If not
>>>>> then CommInterview in the DTFInterview should be created in CDC
>>>>> mode only.
>>>>>
>>>>> And I propose to split constants in two classes with appropriate
>>>>> names: one class for transports' constants and another for modes'
>>>>> constants.
>>>>>
>>>>> Thanks,
>>>>> Alexander
>>>>> [Message sent by forum member 'skavas' (skavas)]
>>>>>
>>>>> http://forums.java.net/jive/thread.jspa?messageID=204836
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> 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
>>>>
>>>

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