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

Read/write to XML using LINQ

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

Problem

This will compile, but I'm thinking that this implementation of read/write is very hard coded and would not be easily adapted to larger situations where large XML documents are being processed or if they are in a different format structure.

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Xml.Linq;
using System.Text;
using System.IO;
using System.Threading.Tasks;

namespace Testing_LINQ_to_XML
{
class Program
{
static string playerName;
static int playerWin = 10;
static int playerLoss = 1;
static int playerTie = 0;

#region Main
static void Main()
{
Console.WriteLine("Enter player Name...");
playerName = Console.ReadLine();

readXML();
writeXML();
readXML();
Exit();
}
#endregion
#region Write
static void writeXML()
{
var path = @"C:\Users\Viper45\Documents\Visual Studio 2013\Projects\Testing LINQ to XML\Testing LINQ to XML\XML Saves\" + playerName + ".xml";

if (System.IO.File.Exists(path)) //Decides if the player has a xml file already
{
//Get data from existing
XDocument file = XDocument.Load(path);
XElement winTemp = new XElement("playerWin", playerWin);
XElement lossTemp = new XElement("playerLoss", playerLoss);
XElement tieTemp = new XElement("playerTie", playerTie);

////delete existing file
File.Delete(Path.Combine(path));

//combine save data with new game
playerWin = playerWin + (int)winTemp;
playerLoss = playerLoss + (int)lossTemp;
playerTie = playerTie + (int)tieTemp;

////Creates new file using last game played.
XDeclaration _obj = new XDeclaration("1.0", "utf-8", "");
XNamespace gameSaves

Solution

this implementation of read/write is very hard coded and would not be easily adapted to larger situations where large XML documents are being processed or if they are in a different format structure.

Those are both different situations than what you have now, so those might require different solutions. But it seems that you're not in such situation, you have small amount of simple data, so you probably shouldn't worry about that (this is called You aren't gonna need it, or YAGNI for short).

If you have data that easily maps directly to classes, XML serialization is going to be useful.

If you have huge amount of data that won't fit into memory at once, consider using the overloads of XElemement.Load() and XElement.Save() that transform between XmlReader/XmlWriter and XElement. (E.g. you would use XmlReader to read the topmost parts of the XML, but then read each of the inner elements using XElement.)

static string playerName;
static int playerWin = 10;
static int playerLoss = 1;
static int playerTie = 0;


This should be enclosed into a Player class, not static fields on Program.

#region Main


I don't see any reason for enclosing each method into its own #region, it just adds noise to your code.

var path = @"C:\Users\Viper45\Documents\Visual Studio 2013\Projects\Testing LINQ to XML\Testing LINQ to XML\XML Saves\" + playerName + ".xml";


This sounds like you want a path relative to the location of the executable in the project directory, something like @"..\XML Saves\" + playerName + ".xml".

In a production application, consider using a subdirectory of AppData (you can get that using Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData)).

System.IO.File.Exists(path)


You're already using System.IO, no need to repeat the namespace here.

Path.Combine(path)


You're not combining the path with anything, just path would work the same.

XElement winTemp = new XElement("playerWin", playerWin);
XElement lossTemp = new XElement("playerLoss", playerLoss);
XElement tieTemp = new XElement("playerTie", playerTie);

…

playerWin = playerWin + (int)winTemp;
playerLoss = playerLoss + (int)lossTemp;
playerTie = playerTie + (int)tieTemp;


I don't understand this code. This is basically just a complicated way to double each of the fields.

XDeclaration _obj = new XDeclaration("1.0", "utf-8", "");
XNamespace gameSaves = "gameSaves";
XElement file = new XElement("Root",
                    new XElement("Player",
                        new XElement("playerName", playerName),
                        new XElement("Stats",
                            new XElement ("playerWin", playerWin),
                            new XElement("playerLoss", playerLoss),
                            new XElement("playerTie", playerTie))));

file.Save(path);


This code is repeated twice, only with different values for playerWin, playerLoss and playerTie. You should extract it into a method that takes those values as parameters, to make your code more DRY.

XElement winTemp = new XElement("playerWin", playerWin);


Consider using var here. It's clear from the right side of the assignment what the type is, so there is no reason to repeat the type on the left side.

Some people (me included) think that var makes sense even when the type isn't clear, but that's a matter of opinion.

var path = @"C:\Users\Viper45\Documents\Visual Studio 2013\Projects\Testing LINQ to XML\Testing LINQ to XML\XML Saves\" + playerName + ".xml";


Another code that's repeated, try to avoid that.

string temp = Console.ReadLine();


You don't need the variable here, just Console.ReadLine(); is enough. (Pressing Ctrl+F5 instead of just F5 in Visual Studio also works.)

Code Snippets

static string playerName;
static int playerWin = 10;
static int playerLoss = 1;
static int playerTie = 0;
#region Main
var path = @"C:\Users\Viper45\Documents\Visual Studio 2013\Projects\Testing LINQ to XML\Testing LINQ to XML\XML Saves\" + playerName + ".xml";
System.IO.File.Exists(path)
Path.Combine(path)

Context

StackExchange Code Review Q#74217, answer score: 6

Revisions (0)

No revisions yet.