incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rohit Yadav <rohit.ya...@citrix.com>
Subject Re: Concern on GenericDaoBase findBy method
Date Sat, 12 Jan 2013 01:13:26 GMT
Thanks Alex! You gotta preach us there is a lot of code and design ideas that we don't understand,
we can only learn from our masters and our mistakes :P

Regards.

On 11-Jan-2013, at 5:03 PM, Alex Huang <Alex.Huang@citrix.com> wrote:

> 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