Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: cloudstack-dev@incubator.apache.org
Received-SPF: pass (nike.apache.org: domain of rohit.yadav@citrix.com
 designates 203.166.19.134 as permitted sender)
From: Rohit Yadav <rohit.yadav@citrix.com>
To: "cloudstack-dev@incubator.apache.org"
	<cloudstack-dev@incubator.apache.org>
Date: Sat, 12 Jan 2013 06:43:26 +0530
Subject: Re: Concern on GenericDaoBase findBy method
Thread-Topic: Concern on GenericDaoBase findBy method
Thread-Index: Ac3wYgdUd7dudtthR+K7faH+nkD37g==
Message-ID: <D5DBAAAC-F15C-40F8-9C03-53CF0E711A06@citrix.com>
References: <679E165C-69D2-40E2-92A6-6D8A6CAD4A3E@citrix.com>
 <B1DF26ECC0458748AC97CECE2DA98D41012FA3B29C1B@SJCPMAILBOX01.citrite.net>
In-Reply-To: 
 <B1DF26ECC0458748AC97CECE2DA98D41012FA3B29C1B@SJCPMAILBOX01.citrite.net>
Accept-Language: en-US
Content-Language: en-US
acceptlanguage: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Thanks Alex! You gotta preach us there is a lot of code and design ideas th=
at 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...
>=20
> Adding checks where the business logic does not dictate it to avoid excep=
tions is something that you should never do.  So what do I mean by this?  T=
ake 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 elsew=
here.  So when should there be checks.  User inputs should always be checke=
d.  When the variable is null means something different then you should che=
ck.
>=20
> You can also read this.
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+log=
ging
>=20
> Exceptions are your friend.  Embrace it, don't avoid it. =20
>=20
> --Alex
>=20
>> -----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
>>=20
>> 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 a=
ny use
>> case? No changes I found till now.
>>=20
>> @@ -905,6 +905,8 @@ public abstract class GenericDaoBase<T, ID extends
>> Serializable> implements Gene
>>     public T findById(final ID id) {
>> +        if (id =3D=3D null)
>> +            return null;
>>         if (_cache !=3D null) {
>>             final Element element =3D _cache.get(id);
>>             return element =3D=3D 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 =3D=3D null)
>> +            return null;
>>         SearchCriteria<T> sc =3D createSearchCriteria();
>>         sc.addAnd("uuid", SearchCriteria.Op.EQ, uuid);
>>=20
>> Regards.
>=20