snippetcsharpMinor
Check if XmlNode with certain attribute exists and create XmlNode if not
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:
Entry Point:
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
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
The
In
In
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.