HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Check if XmlNode with certain attribute exists and create XmlNode if not

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
createwithcertainattributeexistsandchecknotxmlnode

Problem

I'm not going to explain the purpose of the code provided below. If you do not understand it, i know that i have to improve it (naming, structure etc.).

I would like to know how to improve this code in terms of readability and complexity.

Data structure:


  
    
      1
      William
      Shakespeare
      englischer Dramatiker, Lyriker und Schauspieler
      Sonntag, 26. April 1564
      Dienstag, 3. Mai 1616
      /Data/Images/Author1.jpg
    
    
      2
      Friedrich
      Nietzsche
      deutscher Philologe und Philosoph
      Dienstag, 15. Oktober 1844
      Samstag, 25. August 1900
      /Data/Images/Author2.jpg
    
  
  
    
      1
      qwerty
    
    
      2
      qwerty
    
  


Entry Point:

private void SaveAuthor()
{
    var xmlFileHandler = new XmlFileHandler();
    //xmlFileHandler.CreateXmlFile();

    xmlFileHandler.AddAuthor(new Author(1, "William", "Shakespeare", "englischer Dramatiker, Lyriker und Schauspieler",
        new DateTime(1564, 04, 26), new DateTime(1616, 05, 3))); 

    xmlFileHandler.AddAuthor(new Author(2, "Friedrich", "Nietzsche", "deutscher Philologe und Philosoph",
        new DateTime(1844, 10, 15), new DateTime(1900, 08, 25)));
}


XmlFileHandler class

```
public class XmlFileHandler
{
private const string FileName = "quotes.xml";
private const string AuthorsNodeName = "Authors";

public bool AddAuthor(Author author)
{
var xmlQuotes = XDocument.Load(FileName);

var authorExists = CheckIfAuthorAlreadyExists(author.AuthorId, xmlQuotes);

if (!authorExists)
{
this.AddAuthorToXmlDocument(author, xmlQuotes);

return true;
}

return false;
}

private void AddAuthorToXmlDocument(Author author, XDocument xmlQuotes)
{
var authorsNode = xmlQuotes.Descendants(AuthorsNodeName).FirstOrDefault();
if (authorsNode != null) autho

Solution

Instead of reloading the XML file every time you call AddAuthor, it would be more efficient to load the file once. If you are concerned about other threads or processes writing to the file, then you might want to lock it.

The authorExists local variable is pointless, you could drop it. Unless you think the code is more readable this way.

In AddAuthorToXmlDocument, probably you only want to save the file after successfully adding an author, so I would move the saving within the if (authorsNode != null).

In CheckIfAuthorAlreadyExists the final if is unnecessary, you can simply:

return xElements.Any()

Code Snippets

return xElements.Any()

Context

StackExchange Code Review Q#47066, answer score: 6

Revisions (0)

No revisions yet.