Wordy LINQ for XML word list

Question

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
{
 /// <summary>
 /// Interaction logic for MainWindow.xaml
 /// </summary>
 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();
 }
 public class WordList
 {
 string english;
 string romaji;
 string japanese;
 public string English
 {
 get { return english; }
 set { english = value; }
 }
 public string Romaji
 {
 get { return romaji; }
 set { romaji = value; }
 }
 public string Japanese
 {
 get { return japanese; }
 set { japanese = value; }
 }
 }
 }
}

• Mast: You're using a lot. I can't imagine you need all of those.

Answer 1

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<string> 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:

/// <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; }
 }
}

• Jesse Glover: This answer is the most in depth and really explains a great alternative to what I have.
• Jesse Glover: Although there is one slight issue with the code sample. In the GetWords method, it should be... Private List<string> GetWords(Xdocument xdoc, string name)
• Risky Martin: @JesseGlover: Good catch! Fixed.
• BCdotWEB: Prefixing const with "_c"? The Microsoft standard says const should be PascalCase. Methods -- xmlDataLoad() -- should also be PascalCase.
• RobH: It's more common to name methods starting with the verb - e.g. XmlDataLoad sounds a lot better as LoadXmlData

Answer 2

Sure. Create a helper method GetWords that takes XDocument xdoc and string name as parameter:

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

Then xmlDataLoad can become:

 private void xmlDataLoad(XDocument xdoc)
 {
 string xmlPath = @"C:\Users\Jesse\Source\Repos\AsyncSpeechSynth\EJCTest\XMLControlLibrary\newDictionary.xml";
 XDocument xdoc = XDocument.Load(xmlPath);
 engList.ItemsSource = GetWords(xdoc, "English");
 romList.ItemsSource = GetWords(xdoc, "Romaji");
 jpnList.ItemsSource = GetWords(xdoc, "Japanese");
 }