##Naming
Naming
There are .NET Naming Guidelines which state that methods should be named using PascalCase
casing. You haven't done this for buildString()
.
##The code
The code
- The
InnerXml
property returns astring
hence there is no need to callToString()
on the property. - Because all methods are
public
you should do proper parameter validation. At least you should check if the passed parameter isnull
and if yes throw anArgumentNullException
. - The
PrintOutput ()
method does way too much. It builds the output, writes it to the console and reads a line from the console. - Because
_root
won't change you should make itreadonly
. - Calling
PrintOutput
twice results in doubling the printed value because you never reset/clear theStringBuilder
. - The method name of
ReturnNameAsString
is too much. Because the method isn'tvoid
it is clear that it returns something. From looking at the returned type (string
) it is clear what the method returns. Just call itGetNodeName
instead. - The class name
XMLRecursionReader
exposes too much information about the implementation. A user of your code won't need to know that you solve the task by using recursion. - Both methods
buildString
andReturnNameAsString
should be private. - The
if (node.HasChildNodes)
in thebuildString
method is superflous because you are using aforeach
to iterate over theChildNodes
. If there aren't anyChildNodes
theforeach
won't iterate. - The constructor assumes that the passed
XmlDocument
containesChildNodes
which may not be the case.
##Naming
There are .NET Naming Guidelines which state that methods should be named using PascalCase
casing. You haven't done this for buildString()
.
##The code
- The
InnerXml
property returns astring
hence there is no need to callToString()
on the property. - Because all methods are
public
you should do proper parameter validation. At least you should check if the passed parameter isnull
and if yes throw anArgumentNullException
. - The
PrintOutput ()
method does way too much. It builds the output, writes it to the console and reads a line from the console. - Because
_root
won't change you should make itreadonly
. - Calling
PrintOutput
twice results in doubling the printed value because you never reset/clear theStringBuilder
. - The method name of
ReturnNameAsString
is too much. Because the method isn'tvoid
it is clear that it returns something. From looking at the returned type (string
) it is clear what the method returns. Just call itGetNodeName
instead. - The class name
XMLRecursionReader
exposes too much information about the implementation. A user of your code won't need to know that you solve the task by using recursion. - Both methods
buildString
andReturnNameAsString
should be private. - The
if (node.HasChildNodes)
in thebuildString
method is superflous because you are using aforeach
to iterate over theChildNodes
. If there aren't anyChildNodes
theforeach
won't iterate. - The constructor assumes that the passed
XmlDocument
containesChildNodes
which may not be the case.
Naming
There are .NET Naming Guidelines which state that methods should be named using PascalCase
casing. You haven't done this for buildString()
.
The code
- The
InnerXml
property returns astring
hence there is no need to callToString()
on the property. - Because all methods are
public
you should do proper parameter validation. At least you should check if the passed parameter isnull
and if yes throw anArgumentNullException
. - The
PrintOutput ()
method does way too much. It builds the output, writes it to the console and reads a line from the console. - Because
_root
won't change you should make itreadonly
. - Calling
PrintOutput
twice results in doubling the printed value because you never reset/clear theStringBuilder
. - The method name of
ReturnNameAsString
is too much. Because the method isn'tvoid
it is clear that it returns something. From looking at the returned type (string
) it is clear what the method returns. Just call itGetNodeName
instead. - The class name
XMLRecursionReader
exposes too much information about the implementation. A user of your code won't need to know that you solve the task by using recursion. - Both methods
buildString
andReturnNameAsString
should be private. - The
if (node.HasChildNodes)
in thebuildString
method is superflous because you are using aforeach
to iterate over theChildNodes
. If there aren't anyChildNodes
theforeach
won't iterate. - The constructor assumes that the passed
XmlDocument
containesChildNodes
which may not be the case.
##Naming
There are .NET Naming Guidelines which state that methods should be named using PascalCase
casing. You haven't done this for buildString()
.
##The code
- The
InnerXml
property returns astring
hence there is no need to callToString()
on the property. - Because all methods are
public
you should do proper parameter validation. At least you should check if the passed parameter isnull
and if yes throw anArgumentNullException
. - The
PrintOutput ()
method does way too much. It builds the output, writes it to the console and reads a line from the console. - Because
_root
won't change you should make itreadonly
. - Calling
PrintOutput
twice results in doubling the printed value because you never reset/clear theStringBuilder
. - The method name of
ReturnNameAsString
is too much. Because the method isn'tvoid
it is clear that it returns something. From looking at the returned type (string
) it is clear what the method returns. Just call itGetNodeName
instead. - The class name
XMLRecursionReader
exposes too much information about the implementation. A user of your code won't need to know that you solve the task by using recursion. - Both methods
buildString
andReturnNameAsString
should be private. - The
if (node.HasChildNodes)
in thebuildString
method is superflous because you are using aforeach
to iterate over theChildNodes
. If there aren't anyChildNodes
theforeach
won't iterate. - The constructor assumes that the passed
XmlDocument
containesChildNodes
which may not be the case.
##Naming
There are .NET Naming Guidelines which state that methods should be named using PascalCase
casing. You haven't done this for buildString()
.
##The code
- The
InnerXml
property returns astring
hence there is no need to callToString()
on the property. - Because all methods are
public
you should do proper parameter validation. At least you should check if the passed parameter isnull
and if yes throw anArgumentNullException
. - The
PrintOutput ()
method does way too much. It builds the output, writes it to the console and reads a line from the console. - Because
_root
won't change you should make itreadonly
. - Calling
PrintOutput
twice results in doubling the printed value because you never reset/clear theStringBuilder
. - The method name of
ReturnNameAsString
is too much. Because the method isn'tvoid
it is clear that it returns something. From looking at the returned type (string
) it is clear what the method returns. Just call itGetNodeName
instead. - The class name
XMLRecursionReader
exposes too much information about the implementation. A user of your code won't need to know that you solve the task by using recursion. - Both methods
buildString
andReturnNameAsString
should be private. - The
if (node.HasChildNodes)
in thebuildString
method is superflous because you are using aforeach
to iterate over theChildNodes
. If there aren't anyChildNodes
theforeach
won't iterate.
##Naming
There are .NET Naming Guidelines which state that methods should be named using PascalCase
casing. You haven't done this for buildString()
.
##The code
- The
InnerXml
property returns astring
hence there is no need to callToString()
on the property. - Because all methods are
public
you should do proper parameter validation. At least you should check if the passed parameter isnull
and if yes throw anArgumentNullException
. - The
PrintOutput ()
method does way too much. It builds the output, writes it to the console and reads a line from the console. - Because
_root
won't change you should make itreadonly
. - Calling
PrintOutput
twice results in doubling the printed value because you never reset/clear theStringBuilder
. - The method name of
ReturnNameAsString
is too much. Because the method isn'tvoid
it is clear that it returns something. From looking at the returned type (string
) it is clear what the method returns. Just call itGetNodeName
instead. - The class name
XMLRecursionReader
exposes too much information about the implementation. A user of your code won't need to know that you solve the task by using recursion. - Both methods
buildString
andReturnNameAsString
should be private. - The
if (node.HasChildNodes)
in thebuildString
method is superflous because you are using aforeach
to iterate over theChildNodes
. If there aren't anyChildNodes
theforeach
won't iterate. - The constructor assumes that the passed
XmlDocument
containesChildNodes
which may not be the case.
##Naming
There are .NET Naming Guidelines which state that methods should be named using PascalCase
casing. You haven't done this for buildString()
.
##The code
- The
InnerXml
property returns astring
hence there is no need to callToString()
on the property. - Because all methods are
public
you should do proper parameter validation. At least you should check if the passed parameter isnull
and if yes throw anArgumentNullException
. - The
PrintOutput ()
method does way too much. It builds the output, writes it to the console and reads a line from the console. - Because
_root
won't change you should make itreadonly
. - Calling
PrintOutput
twice results in doubling the printed value because you never reset/clear theStringBuilder
. - The method name of
ReturnNameAsString
is too much. Because the method isn'tvoid
it is clear that it returns something. From looking at the returned type (string
) it is clear what the method returns. Just call itGetNodeName
instead. - The class name
XMLRecursionReader
exposes too much information about the implementation. A user of your code won't need to know that you solve the task by using recursion. - Both methods
buildString
andReturnNameAsString
should be private. - The
if (node.HasChildNodes)
in thebuildString
method is superflous because you are using aforeach
to iterate over theChildNodes
. If there aren't anyChildNodes
theforeach
won't iterate.