Fred,

Thanks for this answer.  I guess in a single-threaded scope such as 
@RequestScoped, it's safe to do the work on demand in the first access, but 
it's still clunky to write your own cache.
The multi-threaded laziness adds a level of complexity, and I was looking 
for a solution that hides that; your suggestion is to acknowledge it 
explicitly with a startup phase, which in fact is something I would do for 
this anyway; I just want the code to work correctly in case someone alters 
the graph to have dependencies between the services, which is of course 
what Tim's ConcurrentSingleton does.

The challenge is again to write this without splitting the creation of the 
Map<String,Integer> into a separate @Named construction, without doing the 
parsing in the constructor, and without doing the parsing in the accessor.

@RequestScoped
class Ratings {
   private final Map<String,Integer> ratings;
   @Inject Ratings(HttpServletRequest request) {
     this.ratings = request.parseRatings();  // fictitious bit of HTTP 
request metadata
   }
   public int getRating(String key) { 
     // alternatively, since @RequestScoped is single-threaded
     // if (ratings == null) ratings = request.parseRatings();
     return ratings.get(value); 
   }
}

Thank you,
Leigh.


On Saturday, July 21, 2012 8:24:08 AM UTC-7, Fred Faber wrote:
>
> The notion of avoiding heavyweight work in constructors is largely borne 
> out separating the creation of an object graph from executing nontrivial 
> logic in an app.  The object graph creation is essentially a side-effect of 
> injector creation, and we don't have a good reach into this activity.  This 
> prevents us from inclulding some elementary behavior such as error 
> handling.  More optimistically in the no-error case, we want an app to have 
> control as soon as possible instead of blocking on injector creation, and 
> hence want to keep constructors simple and quick.
>
> Lazy initialization is part of the wider topic of object lifecycle. 
>  There's nothing intrinsic within Guice that directly addresses this (minus 
> to some extent type listeners).   However the 
> Service<http://code.google.com/p/guava-libraries/wiki/ServiceExplained>interface
>  within Guava is a convenient solution.  Specifically:  1) define 
> initialization logic as a Service (most often extending 
> AbstractIdleService<http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/AbstractIdleService.html>);
>  
> 2) bind your services within a multibinder (Set<Service>); and, 3) after 
> your injector is created, iterate over this set and start each service.
>
> For (3), creating a wrapper to do the iteration is convenient so that you 
> can more easily parallelize the initialization.  There's a candidate for 
> this to be released within Guava but I don't know what a timeline is / how 
> concrete it is.
>
> Fred
>
> On Fri, Jul 20, 2012 at 6:37 PM, Leigh Klotz, Jr. 
> <[email protected]>wrote:
>
>> I'd like to get some guidance on correct, clear, and concise 
>> initialization of singleton, read-only access objects
>> wrapping cached run-once computations, with either lazy or eager 
>> initialization.
>>
>> I brought this up peripherally in
>>   
>> http://groups.google.com/forum/?fromgroups#!topic/google-guice/u0V97-FZBTQ
>> but I wanted to avoid taking my attempted threadjacking any further and 
>> am starting a new topic.
>>
>> Here's a sample use case:
>>
>> A Database provides a table of String keys and values.  I'd like to 
>> provide a read-only get(String)->String access
>> object, and I'd like to initialize it safely, so that the resulting 
>> object is thread-safe.  I'd like to express this
>> concisely, in a way that is clear to code readers so they will be 
>> incented to copy the pattern.
>>
>> Lazy init can be done in Java using the static hack, as descrbied by Bob 
>> here
>>   http://blog.crazybob.org/2007/01/lazy-loading-singletons.html
>>   and in Wikipedia here
>>   http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom 
>>
>> but I'd like to do it using Guice, for all the usual reasons, such as so 
>> I can integrate these providers in with normal injection.
>>
>> I hope my problem statement is clear. I'm looking for guidance on the 
>> best solution.
>>
>> Here are three unsatisfactory ideas and one maybe OK one. 
>>
>> Proposal A:
>> Proposal A does the work of reading the database in the constructor.
>>
>> This code is clear, and it's certainly concise, and as near as I can 
>> understand, it's thread-safe.
>>
>> Unfortunately, I belive it isn't correct in Guice because of the problems 
>> associated with doing work in constructors
>> (proxy objects, for example).
>>
>>     @LazySingleton
>>     class CachedThings {
>>       private final Map<String,String> cache;
>>
>>       @Inject CachedThings(DB db) {
>>         this.cache = readCache(db);
>>       }
>>
>>       public String get(String x) {
>>         return cache.get(x);
>>       }
>>
>>       private Map<String,String> readCache(DB db) {
>>         Map<String,String> result = new HashMap<String,String>();
>>         for (Row row : db) { map.add(row.a, row.b); }
>>         return result;
>>       }
>>
>>     }
>>
>> Proposal B:
>>
>> Proposal B splits the data access object from the database reader 
>> operation, moving the database read into its own
>> Provider, where it can operate safely.  This appears to be just as 
>> thread-safe as the previous version, but is
>> considerably less concise.  It also exposes a @Named TypeLiteral that is 
>> not only ugly, but also opens the
>> possibility of someone directly injecting that Map and causing unwanted 
>> expensive database reads.
>>
>>     @LazySingleton
>>     class CachedThings {
>>       private final Map<String,String> cache;
>>
>>       @Inject CachedThings(@Named("hack") Map<String,String> cache) {
>>         this.cache = cache;
>>       }
>>
>>       public String get(String x) {
>>         return cache.get(x);
>>       }
>>
>>       static class MyModule extends AbstractModule {
>>          protected void configure() {
>>            bind(new 
>> TypeLiteral<Map<String,String>>(){/**/}.annotatedWith(Names.named("hack")).toProvider(ThingProvider.class).in(LazySingleton.class);
>>  
>>        
>>          }
>>       }
>>
>>       @LazySingleton 
>>       static class ThingProvider implements Provider<Map<String,String>> {
>>         private final DB db;
>>
>>         @Inject ThingProvider(DB db) {
>>           this.db = db;
>>         }
>>
>>         private Map<String,String> get() {
>>           Map<String,String> result = new HashMap<String,String>();
>>           for (Row row : db) { map.add(row.a, row.b); }
>>           return result;
>>         }
>>     }
>>
>>
>> Proposal C:
>> Proposal C uses @Provides methods so avoid to the problematic verbosity 
>> of TypeLiteral, and indeed the code is cleaner, but we still have the 
>> @Named hack and
>> internal data exposure.  (I've written the MyModule as a static class of 
>> CachedThings, which is questionable, so we might need to deduct a few points
>> for the additional verbosity needed to move the module out.)
>>
>>     @LazySingleton
>>     class CachedThings {
>>       private final Map<String,String> cache;
>>
>>       @Inject CachedThings(@Named("hack") Provider<Map<String,String>> 
>> thingProvider) {
>>         this.cache = thingProvider.get();
>>       }
>>
>>       public String get(String x) {
>>         return cache.get(x);
>>       }
>>
>>       static class MyModule extends AbstractModule {
>>          protected void configure() { }
>>
>>          @LazySingleton @Provides @Named("hack") 
>> Provider<Map<String,String>> getThing(DB db) {
>>            Map<String,String> result = new HashMap<String,String>();
>>            for (Row row : db) { map.add(row.a, row.b); }
>>            return result;
>>         }
>>       }
>>     }
>>
>> Proposal D:
>> I don't know much about injected methods other than that they run after 
>> constructors and the results can't be final.
>> Is this correct with regard to Guice initialization sequence?  Is it safe 
>> for multi-threaded readonly access of the resulting HashMap?
>>
>> Maybe this is the right solution is to use an @Inject setCache(DB) method?
>>
>>     @LazySingleton
>>     class CachedThings {
>>       private Map<String,String> cache;
>>
>>       @Inject CachedThings(...) {
>>         ... other stuff here if necessary ...
>>       }
>>
>>       @Inject void setCache(DB db) {
>>          Map<String,String> result = new HashMap<String,String>();
>>          for (Row row : db) { map.add(row.a, row.b); }
>>          cache = result;
>>       }
>>
>>       public String get(String x) {
>>         return cache.get(x);
>>       }
>>     }
>>
>> Thank you,
>> Leigh.
>>
>>  -- 
>> You received this message because you are subscribed to the Google Groups 
>> "google-guice" group.
>> To view this discussion on the web visit 
>> https://groups.google.com/d/msg/google-guice/-/HUKKPENRVqcJ.
>> To post to this group, send email to [email protected].
>> To unsubscribe from this group, send email to 
>> [email protected].
>> For more options, visit this group at 
>> http://groups.google.com/group/google-guice?hl=en.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups 
"google-guice" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/google-guice/-/TAuXaoHoIlQJ.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/google-guice?hl=en.

Reply via email to