incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Koushik Das <koushik....@citrix.com>
Subject RE: Concern on GenericDaoBase findBy method
Date Sat, 12 Jan 2013 12:27:22 GMT
For internal methods you can put asserts in code if input contracts are not honored.

-Koushik

> -----Original Message-----
> From: Alex Huang [mailto:Alex.Huang@citrix.com]
> Sent: Saturday, January 12, 2013 6:33 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: RE: Concern on GenericDaoBase findBy method
> 
> 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.yadav@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.


Mime
View raw message