geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Blum <jb...@pivotal.io>
Subject Re: [DISCUSS] changes to registerInterest API
Date Fri, 17 Nov 2017 18:03:46 GMT
Personally, I prefer the var args method (registerInterest(T... keys))
myself.  It is way more convenient if I only have a few keys when calling
this method then to have to add the keys to a List, especially for testing
purposes.

But, I typically like to pair that with a registerInterest(Iterable<T>
keys) method
as well.  By having a overloaded Iterable variant, then I can pass in any
Collection type I want (which shouldn't be restricted to just List).  It
also is a simple matter to convert any *Collection* (i.e. *List*, *Set*,
etc) to an array, which can be passed to the var args method.  By using List,
you are implying that "order matters" since a List is a order collection of
elements.

This ("*It might even cause problems of pushing in **multiple different
types.*"), regarding var args, does not even make sense. Technically,
List<T> is no different.  Java's type erasure essentially equates var args
too "Object..." (or Object[]) and the List<T> to List (or a List of Objects,
essentially like if you just did this... List<Object>) So, while the
compiler ensures compile-time type-safety of generics, there is no generics
type-safety guarantees at runtime.



On Fri, Nov 17, 2017 at 9:22 AM, Jason Huynh <jhuynh@pivotal.io> wrote:

> Hi Mike,
>
> The current support for List leads to compilation issues if the region is
> type constrained.  However I think you are suggesting instead of a var args
> method, instead provide a registerInterest(List keys) method?
>
> So far what I am hearing requested is:
> deprecate current "ALL_KEYS" and List passing behavior
> registerInterestAllKeys();
> registerInterest(List<T> keys) instead of a registerInterest(T... keys)
>
> Will anyone ever actually have a List as the key itself? The current and
> suggested changes would not allow it registering for a specific List
> object.
>
>
>
> On Thu, Nov 16, 2017 at 6:50 PM Jacob Barrett <jbarrett@pivotal.io> wrote:
>
> > Geode Native C++ and .NET have:
> >
> >   virtual void registerKeys(const
> > std::vector<std::shared_ptr<CacheableKey>> & keys,
> >                             bool isDurable = false,
> >                             bool getInitialValues = false,
> >                             bool receiveValues = true) = 0;
> >
> >   virtual void unregisterKeys(const
> > std::vector<std::shared_ptr<CacheableKey>> & keys) = 0;
> >
> >   virtual void *registerAllKeys*(bool isDurable = false,
> >                                bool getInitialValues = false,
> >                                bool receiveValues = true) = 0;
> >
> >   virtual void unregisterAllKeys() = 0;
> >
> >   virtual void registerRegex(const std::string& regex,
> >                              bool isDurable = false,
> >                              bool getInitialValues = false,
> >                              bool receiveValues = true) = 0;
> >
> >   virtual void unregisterRegex(const char* regex) = 0;
> >
> > I dislike special values like this so yes please make it go away!
> >
> > -Jake
> >
> >
> > On Thu, Nov 16, 2017 at 5:20 PM Dan Smith <dsmith@pivotal.io> wrote:
> >
> > > I don't really like the regex option - it implies that your keys are
> all
> > > strings. Will any other regular expressions work on non string objects?
> > > registerInterestAllKeys() seems like a better option.
> > >
> > > -Dan
> > >
> > > On Thu, Nov 16, 2017 at 4:34 PM, Michael Stolz <mstolz@pivotal.io>
> > wrote:
> > >
> > > > I don't like the vararg option.
> > > > If i'm maintaining a list of keys i'm interested in, I want to be
> able
> > to
> > > > pass that List in.
> > > > Varargs is a poor substitute. It might even cause problems of pushing
> > in
> > > > multiple different types. Keys must all be of one type for a given
> > > Region.
> > > >
> > > >
> > > > I'm very much in favor of deprecating the ALL_KEYS string in favor of
> > > > something that is typed specially if you refer to ALL_KEYS.
> > > >
> > > >
> > > > If that works, then we don't necessarily need the additional API
> > > > registerInterestAllKeys(). But if ALL_KEYS can't be a special type to
> > get
> > > > over the compilation issues then we should go with the new API.
> > > >
> > > >
> > > >
> > > > --
> > > > Mike Stolz
> > > > Principal Engineer, GemFire Product Lead
> > > > Mobile: +1-631-835-4771 <(631)%20835-4771> <(631)%20835-4771>
> > > >
> > > > On Thu, Nov 16, 2017 at 7:02 PM, Anilkumar Gingade <
> > agingade@pivotal.io>
> > > > wrote:
> > > >
> > > > > +1 Deprecating ALL_KEYS option; I believe this is added before we
> > > > supported
> > > > > regex support.
> > > > >
> > > > >  Doesn't seems like a new API is needed. The regex java doc clearly
> > > > > specifies the effect of ".*".
> > > > >
> > > > > +1 for deprecating list argument; and replacing with new API.
> > > > >
> > > > > -Anil.
> > > > >
> > > > >
> > > > >
> > > > > On Thu, Nov 16, 2017 at 3:36 PM, Jason Huynh <jhuynh@pivotal.io>
> > > wrote:
> > > > >
> > > > > > For GEODE-3813 <https://issues.apache.org/jira/browse/GEODE-3813
> >:
> > > > > Region
> > > > > > registerInterest API usage of type parameters is broken
> > > > > > <https://issues.apache.org/jira/browse/GEODE-3813>
> > > > > >
> > > > > >
> > > > > > The current API to registerInterest allows a special string
token
> > > > > > “ALL_KEYS” to be passed in as the parameter to registerInterest(T
> > > key).
> > > > > > This special token causes the registerInterest to behave similar
> to
> > > > > > registerInterestRegex(“.*”).  As the ticket states, if the
region
> > has
> > > > > been
> > > > > > typed to anything other than Object or String, the usage of
> > > “ALL_KEYS”
> > > > > as a
> > > > > > parameter results in a compilation error.
> > > > > >
> > > > > >
> > > > > > Proposals:
> > > > > >
> > > > > > I would like to deprecate the special string “ALL_KEYS”
and
> > document
> > > a
> > > > > > workaround of using registerInterestRegex(“.*”) or we can
add a
> new
> > > API
> > > > > > called registerInterestAllKeys()
> > > > > >
> > > > > >
> > > > > > I think we should also deprecate passing a List Object of keys
> into
> > > > > > registerInterest.  It has the same compilation restrictions
as
> > > > “ALL_KEYS”
> > > > > > when the region is key constrained/typed.  The reason why List
> > would
> > > be
> > > > > > used is to allow registering multiple keys at once.  Instead,
we
> > can
> > > > add
> > > > > a
> > > > > > new var arg API like registerInterest(T… keys).  This problem
and
> > > > > solution
> > > > > > was also documented in the ticket by the ticket creator (Kirk
> Lund)
> > > > > >
> > > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > -Jason
> > > > > >
> > > > >
> > > >
> > >
> >
>



-- 
-John
john.blum10101 (skype)

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message