Oh man....now, you're going to get me started on something that you don't want to get me started on....but here goes my preaching...
Adding checks where the business logic does not dictate it to avoid exceptions is something that you should never do. So what do I mean by this? Take this example code, here there's no logic case for id or uuid to be null because at the database layer these two must not be null. So in fact, you want an exception to be thrown out so a developer can see this immediately because the only time this can happen is either from bad user input or bad programming mistake. Hiding it would just mean it will be a problem elsewhere. So when should there be checks. User inputs should always be checked. When the variable is null means something different then you should check. You can also read this. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+logging Exceptions are your friend. Embrace it, don't avoid it. --Alex > -----Original Message----- > From: Rohit Yadav [mailto:rohit.ya...@citrix.com] > Sent: Friday, January 11, 2013 4:43 PM > To: cloudstack-dev@incubator.apache.org > Subject: Concern on GenericDaoBase findBy method > > In GenericDaoBase's findById and findByUuid if I added a check to return null > if passed value is null to avoid potential exceptions, will this break any use > case? No changes I found till now. > > @@ -905,6 +905,8 @@ public abstract class GenericDaoBase<T, ID extends > Serializable> implements Gene > public T findById(final ID id) { > + if (id == null) > + return null; > if (_cache != null) { > final Element element = _cache.get(id); > return element == null ? lockRow(id, null) : > (T)element.getObjectValue(); > @@ -916,6 +918,8 @@ public abstract class GenericDaoBase<T, ID extends > Serializable> implements Gene > public T findByUuid(final String uuid) { > + if (uuid == null) > + return null; > SearchCriteria<T> sc = createSearchCriteria(); > sc.addAnd("uuid", SearchCriteria.Op.EQ, uuid); > > Regards.
