patterncsharpMinor
Return value when the requested variable returns null
Viewed 0 times
thereturnnullvaluereturnswhenvariablerequested
Problem
I have a method that returns a list from a HTML page:
I'm not sure what to do with the return value in case the
If I return
I'd be happy to hear your ideas and advice.
private List GetRelevantNodes(HtmlDocument aHtmlDoc, string aID)
{
List nodes = new List();
HtmlNode currentDiv = aHtmlDoc.GetElementbyId(aID);
if (currentDiv == null)
return null;
else
//Do stuff - fill the nodes list...
return nodes;
}I'm not sure what to do with the return value in case the
currentDiv variable is null. Obviously I can't continue the method regularly and I need to return from this method with some indication that there's no such div. If I return
null and handle it up the stack it'll work fine but it's less readable code (at list in my opinion... don't you think?). Another thing I can add is an out string with the error message and maybe use it later for the end user output but that still doesn't solve my readable problem. I'd be happy to hear your ideas and advice.
Solution
You're right, this is not the best thing to do. The correct way would be to return an empty list.
Now why is that better than returning
Always. Now if you'd return an empty list, the checking becomes optional on the client side:
Or with checking:
You should always write XML Documentation.
There's a typo, it should be
Also your variables are named not optimal. Drop that
The name of the function is also not good, it's called
private List GetNodes(HtmlDocument aHtmlDoc, string aID)
{
List nodes = new List();
HtmlNode currentDiv = aHtmlDoc.GetElementbyId(aID);
if (currentDiv != null)
{
//Do stuff - fill the nodes list...
}
return nodes;
}Now why is that better than returning
null? Simple, it allows to remove checking from the client code. Imagine your function would return null if there are no nodes, clients always need to write the following code when interacting with it:List nodes = GetNodes(htmlDoc, id);
if (nodes != null)
{
foreach (HtmlNode node in nodes)
{
// Super secret business code
}
}Always. Now if you'd return an empty list, the checking becomes optional on the client side:
foreach (HtmlNode node in GetNodes(htmlDoc, id))
{
// Still secret...
}Or with checking:
List nodes = GetNodes(htmlDoc, id);
if (nodes.Length > 0)
{
foreach (HtmlNode node in nodes)
{
// Super secret business code
}
}You should always write XML Documentation.
private List GetNodes(HtmlDocument aHtmlDoc, string aID)There's a typo, it should be
HtmlNode.Also your variables are named not optimal. Drop that
a prefix, nobody cares if it is an argument or not, it's a variable you have to work with.The name of the function is also not good, it's called
GetNodes yet from what I see (and interpret into it) it does not fetch the nodes with the given ID, it fetches the children of the first node with the given ID. So it should actually be called GetChildren.Code Snippets
private List<HtmlNodes> GetNodes(HtmlDocument aHtmlDoc, string aID)
{
List<HtmlNode> nodes = new List<HtmlNode>();
HtmlNode currentDiv = aHtmlDoc.GetElementbyId(aID);
if (currentDiv != null)
{
//Do stuff - fill the nodes list...
}
return nodes;
}List<HtmlNode> nodes = GetNodes(htmlDoc, id);
if (nodes != null)
{
foreach (HtmlNode node in nodes)
{
// Super secret business code
}
}foreach (HtmlNode node in GetNodes(htmlDoc, id))
{
// Still secret...
}List<HtmlNode> nodes = GetNodes(htmlDoc, id);
if (nodes.Length > 0)
{
foreach (HtmlNode node in nodes)
{
// Super secret business code
}
}private List<HtmlNodes> GetNodes(HtmlDocument aHtmlDoc, string aID)Context
StackExchange Code Review Q#41867, answer score: 8
Revisions (0)
No revisions yet.