You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lenya.apache.org by Michael Wechner <mi...@wyona.com> on 2006/03/21 11:19:09 UTC
Re: Warning logs
Andreas Hartmann wrote:
> michi@apache.org wrote:
>
> [...]
>
>> protected synchronized Node findNode(Node node, List ids) {
>
>
> [...]
>
>> if (node != null) {
>> treeNode = new SiteTreeNodeImpl(node);
>> ContainerUtil.enableLogging(treeNode, getLogger());
>> + } else {
>> + log.warn("No such node: " + documentId);
>> }
>
>
> It is not appropriate to log a warning here.
> The javadocs clearly state that this method returns null if the
> node doesn't exist:
well, I think the warning is appropriate if one actually tries
to receive this node and it doesn't exist.
The problem might be that one should rather use an "exist()" method
which returns true or false and one doesn't have to throw a warning ....
Michi
>
> /**
> * Return the Node for a given document-id.
> *
> * @param documentId the document-id of the node that is requested
> *
> * @return a <code>SiteTreeNode</code> if there is a node for the
> * given document-id, null otherwise.
> */
>
>
>
> -- Andreas
>
>
--
Michael Wechner
Wyona - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
michael.wechner@wyona.com michi@apache.org
+41 44 272 91 61
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org
Re: Warning logs
Posted by Andreas Hartmann <an...@apache.org>.
Michael Wechner wrote:
> Andreas Hartmann wrote:
>
>> michi@apache.org wrote:
>>
>> [...]
>>
>>> protected synchronized Node findNode(Node node, List ids) {
>>
>>
>> [...]
>>
>>> if (node != null) {
>>> treeNode = new SiteTreeNodeImpl(node);
>>> ContainerUtil.enableLogging(treeNode, getLogger());
>>> + } else {
>>> + log.warn("No such node: " + documentId);
>>> }
>>
>>
>> It is not appropriate to log a warning here.
>> The javadocs clearly state that this method returns null if the
>> node doesn't exist:
>
>
> well, I think the warning is appropriate if one actually tries
> to receive this node and it doesn't exist.
If the client code doesn't check if the returned node is null,
then it's a bug in the client code. The javadoc is very clear about that.
There are several options for non-existing items in getter methods:
a) return null and state this in the javadocs
b) return a NullObject (questionable practice)
c) throw an exception (this requires an additional exists() method)
Everything else is IMO bad style.
> The problem might be that one should rather use an "exist()" method
> which returns true or false and one doesn't have to throw a warning ....
Yes, see above. In this case the getter must throw an exception.
-- Andreas
--
Andreas Hartmann
Wyona Inc. - Open Source Content Management - Apache Lenya
http://www.wyona.com http://lenya.apache.org
andreas.hartmann@wyona.com andreas@apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lenya.apache.org
For additional commands, e-mail: dev-help@lenya.apache.org