Return-Path: Mailing-List: contact torque-dev-help@db.apache.org; run by ezmlm Delivered-To: mailing list torque-dev@db.apache.org Received: (qmail 60910 invoked from network); 30 Jun 2003 22:42:29 -0000 Received: from dsl027-176-145.sfo1.dsl.speakeasy.net (HELO despot) (216.27.176.145) by daedalus.apache.org with SMTP; 30 Jun 2003 22:42:29 -0000 Received: by despot (Postfix, from userid 500) id C28CD3483D8; Mon, 30 Jun 2003 15:42:45 -0700 (PDT) Sender: dlr@finemaltcoding.com To: "Turbine Torque Developers List" Subject: Re: cvs commit: db-torque/src/java/org/apache/torque TorqueSingleton.java References: <20030630221105.99907.qmail@icarus.apache.org> From: Daniel Rall Date: 30 Jun 2003 15:42:45 -0700 In-Reply-To: <20030630221105.99907.qmail@icarus.apache.org> Message-ID: Lines: 101 User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Common Lisp) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N I like the improved the diagnostics! A few other comments below. henning@apache.org writes: ... > - replaced some while() loops with iterators with > for(Iterator ...; it.hasNext();) loops > - Added clear warnings for > - databases configured but no adapters set > - datasource configured but no factories set > - no databases at all configured ... > --- TorqueSingleton.java 24 Jun 2003 09:43:29 -0000 1.6 > +++ TorqueSingleton.java 30 Jun 2003 22:11:05 -0000 1.7 ... > @@ -261,8 +263,13 @@ > DB db = DBFactory.create(adapter); > // register the adapter for this name > adapterMap.put(handle, db); > + foundAdapters = true; > } > } > + if (!foundAdapters) > + { > + log.warn("Databases defined but no adapters configurations found!"); > + } "adapter configurations" would be better than "adapters configurations". > } > catch (Exception e) > { > @@ -272,8 +279,9 @@ > } > else > { > - log.warn("There were no adapters in the configuration."); > + log.warn("No Database definitions found!"); > } > + > } > > /** If a user is hit by this one, they could probably use clue as to where to define their configuration. How about adding reference to a web page with help docs or to a configuration file? ... > @@ -307,8 +316,13 @@ > (DataSourceFactory) dsfClass.newInstance(); > dsf.initialize(c.subset(handle)); > dsFactoryMap.put(handle, dsf); > + foundFactories = true; > } > } > + if (!foundFactories) > + { > + log.warn("Data Sources configured but no factories found!"); > + } > } > catch (Exception e) > { Would a default factory be usable here (e.g. like in the case where they're using a default data source type)? > @@ -406,11 +420,9 @@ > int pref = Torque.MANAGER_PREFIX.length(); > int suff = Torque.MANAGER_SUFFIX.length(); > > - Iterator keys = conf.getKeys(); > - > - while (keys.hasNext()) > + for (Iterator it = conf.getKeys(); it.hasNext();) > { > - String key = (String) keys.next(); > + String key = (String) it.next(); > > if (key.startsWith(Torque.MANAGER_PREFIX) > && key.endsWith(Torque.MANAGER_SUFFIX)) "keys" is a slightly more descriptive variable name than "it". > @@ -562,10 +574,9 @@ > { > if (dbMaps != null) > { > - Iterator maps = dbMaps.values().iterator(); > - while (maps.hasNext()) > + for (Iterator it = dbMaps.values().iterator(); it.hasNext();) > { > - DatabaseMap map = (DatabaseMap) maps.next(); > + DatabaseMap map = (DatabaseMap) it.next(); > IDBroker idBroker = map.getIDBroker(); > if (idBroker != null) > { "maps" is a more descriptive variable name than "it". -- Daniel Rall