The 2 options you listed are indeed better, but they don't work if your goal
is to prevent access to an uninitialized FetchOptions object. That was our
original intent, so that's how we ended up steering you through the Builder
at the start. At one point before we launched we had a more standard
Builder that you had to call build() on at the end. I liked it because it
meant that all the FetchOptions members could be final, but there was some
debate within the team and we ultimately decided that in this case
conciseness trumps immutability, so we dropped the explicit build() call and
moved the setters to FetchOptions itself. We also considered putting the
"with" constructors directly on FetchOptions and getting rid of the Builder
entirely, but we decided that a static method named
FetchOptions.withLimit(int limit) and an instance method named
FetchOptions.limit(limit) wouldn't give enough of a hint about the intended
usage. So here we are.
API design is hard.
Max
On Thu, Apr 1, 2010 at 12:14 PM, Jeff Schnitzer <j...@infohazard.org> wrote:
> Oh, I wasn't suggesting a constructor that takes all the arguments,
> I'm just wondering why this:
>
> FetchOptions opts = FetchOptions.Builder.withLimit(10).offset(200);
>
> ...is better than this...
>
> FetchOptions opts = new FetchOptions().limit(10).offset(200);
>
> ...or even...
>
> FetchOptions opts = FetchOptions.create().limit(10).offset(200);
>
> Adding the Builder with duplicate-but-differently-named methods seems
> awkward.
>
> Jeff
>
> On Thu, Apr 1, 2010 at 11:43 AM, Max Ross (Google)
> <maxr+appeng...@google.com <maxr%2bappeng...@google.com>> wrote:
> > Ok, we'll expose that method.
> >
> > With a public constructor it's very easy to mix up your args, especially
> > when you have args of the same type. For example, someone is pretty much
> > guaranteed to mix up the offset, limit, and prefetch args. A Builder
> > prevents this. Josh Bloch has a nice chapter on this in the 2nd edition
> of
> > Effective Java.
> >
> > Max
> >
> > On Thu, Apr 1, 2010 at 11:10 AM, Jeff Schnitzer <j...@infohazard.org>
> wrote:
> >>
> >> Yes, that would solve the problem. Although I have to wonder, how is
> >> this any better than having a public constructor?
> >>
> >> Jeff
> >>
> >> On Thu, Apr 1, 2010 at 10:08 AM, Max Ross (Google)
> >> <maxr+appeng...@google.com <maxr%2bappeng...@google.com>> wrote:
> >> > Hi Jeff,
> >> >
> >> > Note that DatastoreServiceConfig exposes a withDefaults() method.
> >> > FetchOptions has one too but it's package protected. We didn't expose
> >> > it
> >> > because FetchOptions isn't required for many low level datastore calls
> >> > and
> >> > we didn't see why users would want to create one if they didn't plan
> to
> >> > adjust the offset, the limit, the prefetch size, etc. We didn't
> >> > consider
> >> > your use case though, and it makes perfect sense. Unless there's some
> >> > other
> >> > aspect of the Builder pattern that is getting in your way that you
> have
> >> > explained, I think we could easily address your concern if we exposed
> >> > FetchOptions.withDefaults(). Is that right?
> >> >
> >> > Thanks,
> >> > Max
> >> >
> >> >
> >> > On Thu, Apr 1, 2010 at 3:07 AM, Jeff Schnitzer <j...@infohazard.org>
> >> > wrote:
> >> >>
> >> >> This is a genuine, heartfelt plea: The Builder pattern
> >> >> (DatastoreServiceConfig, FetchOptions) makes code extra annoying when
> >> >> layering an API on top of the low-level API.
> >> >>
> >> >> Let's say you are writing some code by hand that creates a
> >> >> FetchOptions with a limit and an offset:
> >> >>
> >> >> FetchOptions opts = FetchOptions.Builder.withLimit(100).offset(20);
> >> >>
> >> >> Pretty straightforward when typing it out by hand, but the lack of
> >> >> orthogonality between withLimit() and limit() is a PITA when you're
> >> >> trying to automate the creation of a builder. The problem is, you
> >> >> can't start with a "blank" FetchOptions. The result is we end up
> >> >> writing lots of code like this:
> >> >>
> >> >> if (limit != null)
> >> >> {
> >> >> if (opts == null)
> >> >> opts = FetchOptions.Builder.withLimit(limit);
> >> >> else
> >> >> opts = opts.limit(limit);
> >> >> }
> >> >>
> >> >> This wouldn't be necessary if FetchOptions.Builder had a "create()"
> >> >> method that would produce a blank FetchOptions. Or if FetchOptions
> >> >> had a public constructor. Or if FetchOptions was an interface or
> even
> >> >> a nonfinal class.
> >> >>
> >> >> I notice that DatastoreServiceConfig is following in the path of
> >> >> FetchOptions. My initial reaction was Noooooo! :-)
> >> >>
> >> >> Thanks for listening,
> >> >> Jeff
> >> >>
> >> >> --
> >> >> You received this message because you are subscribed to the Google
> >> >> Groups
> >> >> "Google App Engine for Java" group.
> >> >> To post to this group, send email to
> >> >> google-appengine-j...@googlegroups.com.
> >> >> To unsubscribe from this group, send email to
> >> >> google-appengine-java+unsubscr...@googlegroups.com<google-appengine-java%2bunsubscr...@googlegroups.com>
> .
> >> >> For more options, visit this group at
> >> >> http://groups.google.com/group/google-appengine-java?hl=en.
> >> >>
> >> >
> >> > --
> >> > You received this message because you are subscribed to the Google
> >> > Groups
> >> > "Google App Engine for Java" group.
> >> > To post to this group, send email to
> >> > google-appengine-j...@googlegroups.com.
> >> > To unsubscribe from this group, send email to
> >> > google-appengine-java+unsubscr...@googlegroups.com<google-appengine-java%2bunsubscr...@googlegroups.com>
> .
> >> > For more options, visit this group at
> >> > http://groups.google.com/group/google-appengine-java?hl=en.
> >> >
> >>
> >> --
> >> You received this message because you are subscribed to the Google
> Groups
> >> "Google App Engine for Java" group.
> >> To post to this group, send email to
> >> google-appengine-j...@googlegroups.com.
> >> To unsubscribe from this group, send email to
> >> google-appengine-java+unsubscr...@googlegroups.com<google-appengine-java%2bunsubscr...@googlegroups.com>
> .
> >> For more options, visit this group at
> >> http://groups.google.com/group/google-appengine-java?hl=en.
> >>
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Google App Engine for Java" group.
> > To post to this group, send email to
> google-appengine-j...@googlegroups.com.
> > To unsubscribe from this group, send email to
> > google-appengine-java+unsubscr...@googlegroups.com<google-appengine-java%2bunsubscr...@googlegroups.com>
> .
> > For more options, visit this group at
> > http://groups.google.com/group/google-appengine-java?hl=en.
> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "Google App Engine for Java" group.
> To post to this group, send email to
> google-appengine-j...@googlegroups.com.
> To unsubscribe from this group, send email to
> google-appengine-java+unsubscr...@googlegroups.com<google-appengine-java%2bunsubscr...@googlegroups.com>
> .
> For more options, visit this group at
> http://groups.google.com/group/google-appengine-java?hl=en.
>
>
--
You received this message because you are subscribed to the Google Groups
"Google App Engine for Java" group.
To post to this group, send email to google-appengine-j...@googlegroups.com.
To unsubscribe from this group, send email to
google-appengine-java+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/google-appengine-java?hl=en.
