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

Wordy LINQ for XML word list

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

Problem

This code works as intended, however, I am sure that there is a way to condense the xmldataLoad method. If someone could maybe explain a better approach than what I have done with the method, I would be extremely grateful.
Essentially, what I am saying is I am rather new to using LINQ and don't fully understand the various ways of using it, and I am quite sure there are many people in the same boat as I am.

```
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;
using System.Xml.Linq;
using System.Xml.Serialization;
//using XMLControlLibrary;

namespace EJCTest
{
///
/// Interaction logic for MainWindow.xaml
///
public partial class MainWindow : Window
{

public MainWindow()
{
InitializeComponent();
}

private void xmlDataLoad()
{
string xmlPath = @"C:\Users\Jesse\Source\Repos\AsyncSpeechSynth\EJCTest\XMLControlLibrary\newDictionary.xml";
XDocument xdoc = XDocument.Load(xmlPath);

var eng = from wordList in xdoc.Root.Elements("Word")
select wordList.Element("English").Value;
engList.ItemsSource = eng.ToList();

var rom = from wordList in xdoc.Root.Elements("Word")
select wordList.Element("Romaji").Value;
romList.ItemsSource = rom.ToList();

var jpn = from wordList in xdoc.Root.Elements("Word")
select wordList.Element("Japanese").Value;
jpnList.ItemsSource = jpn.ToList();
}

private void Button_Click(object sender, RoutedEventArgs e)
{
xmlDataLoad();

Solution

Your LINQ is fine. All you're doing is simple navigation of an XDocument. There's isn't any more you can do with LINQ there. However, there are still some improvements that can be made.

Borrowing from janos' answer, you can create a helper method GetWords that takes XDocument xdoc and string name as parameters:

private List GetWords(XDocument xdoc, string name)
{
    var words = from wordList in xdoc.Root.Elements("Word")
                select wordList.Element(name).Value;
    return words.ToList();
}


You can store the XML path as a constant at the class level:

private const string _cXmlPath = @"C:\Users\Jesse\Source\Repos\AsyncSpeechSynth\EJCTest\XMLControlLibrary\newDictionary.xml";


You can get rid of magic strings.

You can use auto properties.

Edit: You can rename the xmlDataLoad method using UpperCamelCase and to start with a verb. Thanks @BCdotWEB and @RobH.

The new version of your class looks like this:

/// 
/// Interaction logic for MainWindow.xaml
/// 
public partial class MainWindow : Window
{
    private const string _cXmlPath = @"C:\Users\Jesse\Source\Repos\AsyncSpeechSynth\EJCTest\XMLControlLibrary\newDictionary.xml";
    private const string _cWordName = "Word";
    private const string _cEnglishName = "English";
    private const string _cRomajiName = "Romaji";
    private const string _cJapaneseName = "Japanese";

    public MainWindow()
    {
        InitializeComponent();
    }

    private void LoadXmlData()
    {            
        XDocument xdoc = XDocument.Load(_cXmlPath);

        engList.ItemsSource = GetWords(xdoc, _cEnglishName);
        romList.ItemsSource = GetWords(xdoc, _cRomajiName);
        jpnList.ItemsSource = GetWords(xdoc, _cJapaneseName);
    }

    private List GetWords(XDocument xdoc, string name)
    {
        var words = from wordList in xdoc.Root.Elements(_cWordName)
                    select wordList.Element(name).Value;
        return words.ToList();
    }

    private void Button_Click(object sender, RoutedEventArgs e)
    {
        LoadXmlData();
    }

    public class WordList
    {
        public string English { get; set; }
        public string Romaji { get; set; }
        public string Japanese { get; set; }
    }
}

Code Snippets

private List<string> GetWords(XDocument xdoc, string name)
{
    var words = from wordList in xdoc.Root.Elements("Word")
                select wordList.Element(name).Value;
    return words.ToList();
}
private const string _cXmlPath = @"C:\Users\Jesse\Source\Repos\AsyncSpeechSynth\EJCTest\XMLControlLibrary\newDictionary.xml";
/// <summary>
/// Interaction logic for MainWindow.xaml
/// </summary>
public partial class MainWindow : Window
{
    private const string _cXmlPath = @"C:\Users\Jesse\Source\Repos\AsyncSpeechSynth\EJCTest\XMLControlLibrary\newDictionary.xml";
    private const string _cWordName = "Word";
    private const string _cEnglishName = "English";
    private const string _cRomajiName = "Romaji";
    private const string _cJapaneseName = "Japanese";

    public MainWindow()
    {
        InitializeComponent();
    }

    private void LoadXmlData()
    {            
        XDocument xdoc = XDocument.Load(_cXmlPath);

        engList.ItemsSource = GetWords(xdoc, _cEnglishName);
        romList.ItemsSource = GetWords(xdoc, _cRomajiName);
        jpnList.ItemsSource = GetWords(xdoc, _cJapaneseName);
    }

    private List<string> GetWords(XDocument xdoc, string name)
    {
        var words = from wordList in xdoc.Root.Elements(_cWordName)
                    select wordList.Element(name).Value;
        return words.ToList();
    }

    private void Button_Click(object sender, RoutedEventArgs e)
    {
        LoadXmlData();
    }

    public class WordList
    {
        public string English { get; set; }
        public string Romaji { get; set; }
        public string Japanese { get; set; }
    }
}

Context

StackExchange Code Review Q#95750, answer score: 4

Revisions (0)

No revisions yet.