Re: [cp-patches] [RFA/JDWP] ArrayReferenceCommandSet.java

[Aaron Luchko]

On Thu, 2005-07-21 at 18:18 -0600, Tom Tromey wrote:
> >>>>> "Aaron" == Aaron Luchko <address@hidden> writes:
> 
> Aaron> There's a slightly ugly part with a big if/else in executeGetValues but
> Aaron> there doesn't seem to be a way to get rid of this.
> 
> This looks really similar to some code in the Values patch.  I suggest
> a static utility method somewhere that takes a class and returns the
> corresponding tag.  Then the big if/else here just becomes a method
> call, and the conditional tag stuff in that method in Values becomes a
> single conditional write... what do you think?

I considered that a bit but decided against it. The code in Value that
does a similar part (writes a tag according to the class) is in
writeValue and is used by both writeTaggedValue and writeUntaggedValue,
and it must both have the option to write the tag (untagged values don't
write tags), and write the value itself. Both these actions depend on
which Class it is, if we had a single method to get tag based on class
we'd still need a switch afterward to write the value for Values.

In order to keep it a single if/else one would need another method in
Value, something like

writeTagAndValue(DataOutputStream os, Object obj, Class type, boolean
tagged, boolean value)

obj would be required since when writing a value the object is required,
type would be for arrayregions since we don't have an object to get the
class from, we'd need to hand it the class directly, tagged is a switch
on whether or not to write the tag for tagged/untagged values and value
is a switch on whether or not to write the obj since we don't want that
for the arrayregions usage (though we could just pass a null object to
signify that instead).

I felt that this added complexity in order to re-use the switch wasn't
worth it since ArrayReference is the only time we use arrayregions in
the entire protocol and I'm pretty sure only time other than Values
we'll need to write out a tag based on a Class. If you feel one of these
solutions is better (or if you think of a better sol'n) I'm glad to
implement it but otherwise I think this approach with the one seemingly
extra if/else ends up being the simplest.

> Aaron> +    // Write all the values, primatives should be untagged and 
> Objects must be
> 
> Should be "primitives" ... this occurs both here and I think in the
> Values patch as well.

Oops, I sometimes have the occasional spellang error :)
patch with gooder spelling attached.

Aaron

jdwpArrayRef2.patch
Description: Text Data