Resx Editor in WPF

Question

To aid in the localization of .resx files, I made a generic resx comparer that allows you to edit the values of the resx's. Also, this is my first proper adventure in WPF (I've just been using Windows-runtime before).

This is my view interface:

public interface IResxTranslationHelperWindow
{
    object DataContext { get; set; }
    event EventHandler<DataGridCellEditEndingEventArgs> EndCellEdit;
    event EventHandler<ResxValues> DeleteRow;

    void Show();
}

The view itself:

<Grid Name="MainGrid">
    <Grid.RowDefinitions>
        <RowDefinition Height="40" />
        <RowDefinition />
    </Grid.RowDefinitions>
    <StackPanel Orientation="Horizontal">
        <StackPanel.Resources>
            <converters:WindowWidthToHalf x:Key="WindowWidthToHalfConverter" />
        </StackPanel.Resources>
        <Button Content="{x:Static properties:Resources.FilePickerButton_BaseResx}" Command="{Binding PickFile}" CommandParameter="DefaultResx"
                Width="{Binding ElementName=MainGrid, Path=ActualWidth, Converter={StaticResource WindowWidthToHalfConverter}}" />
        <Button Content="{x:Static properties:Resources.FilePickerButton_LocalizedResx}" Command="{Binding PickFile}"
                Width="{Binding ElementName=MainGrid, Path=ActualWidth, Converter={StaticResource WindowWidthToHalfConverter}}" />
    </StackPanel>
    <DataGrid ItemsSource="{Binding ResxStrings}" LoadingRow="GridDisplay_LoadingRow" CanUserSortColumns="True" 
              CanUserAddRows="True" CanUserDeleteRows="True" ColumnWidth="300" AutoGenerateColumns="False" BeginningEdit="DataGrid_BeginningEdit"
              CellEditEnding="GridDisplay_CellEditEnding" Grid.Row="1" PreviewKeyDown="DataGrid_PreviewKeyDown">
        <DataGrid.Columns>
            <DataGridTextColumn Binding="{Binding Path=Key}" Header="{x:Static properties:Resources.DataGridHeader_Key_Header}" CanUserSort="True" />
            <DataGridTextColumn Binding="{Binding Path=Value}" Header="{x:Static properties:Resources.DataGridHeader_Value_Header}" CanUserSort="True" />
            <DataGridTextColumn Binding="{Binding Path=LocalizedValue}" Header="{x:Static properties:Resources.DataGridHeader_LocalizedValue_Header}" CanUserSort="True" />
        </DataGrid.Columns>
    </DataGrid>
</Grid>

And the code-behind:

public ResxTranslationHelperWindow()
{
    InitializeComponent();
}

private void GridDisplay_LoadingRow(object sender, DataGridRowEventArgs e)
{
    var item = e.Row.Item as ResxValues;
    if (item == null)
    {
        e.Row.Background = new SolidColorBrush(Colors.White);
        return;
    }

    if (string.IsNullOrEmpty(item.LocalizedValue))
    {
        e.Row.Background = new SolidColorBrush(Colors.LightSeaGreen);
    }
    if (string.IsNullOrEmpty(item.Value))
    {
        e.Row.Background = new SolidColorBrush(Colors.Red);
    }

    if (!string.IsNullOrEmpty(item.Value) && !string.IsNullOrEmpty(item.LocalizedValue))
    {
        e.Row.Background = new SolidColorBrush(Colors.White);
    }
}

private void DataGrid_BeginningEdit(object sender, DataGridBeginningEditEventArgs e)
{
    // keys must not be edited unless they are empty, which signifies adding a new value
    if (e.Column.DisplayIndex == 0)
    {
        var originalText = ((TextBlock) e.EditingEventArgs.OriginalSource).Text;
        if (originalText != "")
        {
            e.Cancel = true;
        }
    }
}

private void GridDisplay_CellEditEnding(object sender, DataGridCellEditEndingEventArgs e)
{
    if (e.EditAction == DataGridEditAction.Commit)
    {
        try
        {
            OnEndCellEdit(e);
        }
        catch (ArgumentException)
        {
            var grid = (DataGrid)sender;
            var itemsLastRemoved = grid.ItemsSource.OfType<ResxValues>()
                    .ToList();
            itemsLastRemoved.RemoveAt(itemsLastRemoved.Count - 1);
            grid.ItemsSource = new ObservableCollection<ResxValues>(itemsLastRemoved);
            grid.SelectedItem = itemsLastRemoved.Last();
        }
    }
}

private void DataGrid_PreviewKeyDown(object sender, KeyEventArgs e)
{
    if (Key.Delete == e.Key)
    {
        var grid = sender as DataGrid;
        OnRowDeleted(grid.SelectedItem as ResxValues);
    }
}

public event EventHandler<DataGridCellEditEndingEventArgs> EndCellEdit;
protected virtual void OnEndCellEdit(DataGridCellEditEndingEventArgs e)
{
    var handler = EndCellEdit;
    if (handler != null)
    {
        handler(this, e);
    }
}

public event EventHandler<ResxValues> DeleteRow;
protected virtual void OnRowDeleted(ResxValues e)
{
    var handler = DeleteRow;
    if (handler != null)
    {
        handler(this, e);
    }
}

My VM:

private readonly IResxTranslationHelperWindow _window;
private XElement _data;
private XElement _localizedData;

public ObservableCollection<ResxValues> ResxStrings { get; set; }
public string DataPath { get; set; }
public string LocalizedDataPath { get; set; }

private ICommand _pickFile;
public ICommand PickFile
{
    get
    {
        return _pickFile ?? (_pickFile = new RelayCommand
        (
            param =>
            {
                var filePath = OpenFilePicker();
                if (filePath == "") { return; }

                if (param as string == "DefaultResx")
                {
                    DataPath = filePath;
                }
                else
                {
                    LocalizedDataPath = filePath;
                }
                LoadData();
            }
        ));
    }
}

public ResxTranslationHelperVM(IResxTranslationHelperWindow window)
{
    _window = window;
    ResxStrings = new ObservableCollection<ResxValues>();

    _window.EndCellEdit += EndCellEdit;
    _window.DeleteRow += DeleteRow;

    window.DataContext = this;
}

private string OpenFilePicker()
{
    var filePicker = new OpenFileDialog
    {
        Multiselect = false,
        SupportMultiDottedExtensions = true,
        CheckPathExists = true,
        Filter = @"Resx Files|*.resx"
    };

    filePicker.ShowDialog();
    return filePicker.FileName;
}

private void EndCellEdit(object sender, DataGridCellEditEndingEventArgs e)
{
    var key = e.Column.DisplayIndex == 0
              ? ((TextBox)e.EditingElement).Text
              : ResxStrings[e.Row.GetIndex()].Key;

    var value = e.Column.DisplayIndex == 1
              ? ((TextBox)e.EditingElement).Text
              : ResxStrings[e.Row.GetIndex()].Value;

    var localizedValue = e.Column.DisplayIndex == 2
                       ? ((TextBox)e.EditingElement).Text
                       : ResxStrings[e.Row.GetIndex()].LocalizedValue;

    if (e.Row.Item as ResxValues == ResxStrings.Last() &&
        ResxStrings.Select(s => s.Key).Contains(key))
    {
        MessageBox.Show(Resources.EndCellEdit_KeyAlreadyExists, Resources.EndCellEdit_AddValueError, MessageBoxButtons.OK, MessageBoxIcon.Warning);
        throw new ArgumentException("Invalid data");
    }

    RemoveNodes(ResxStrings[e.Row.GetIndex()].Key);

    AddNode(_data, key, value);
    AddNode(_localizedData, key, localizedValue);

    SaveFiles();
}

private void DeleteRow(object sender, ResxValues e)
{
    RemoveNodes(e.Key);
    SaveFiles();
    ResxStrings.Remove(e);
}

private void SaveFiles()
{
    _data.Save(DataPath);
    _localizedData.Save(LocalizedDataPath);
}

private void RemoveNodes(string key)
{
    var node = _localizedData.Nodes().OfType<XElement>().FirstOrDefault(n => n.FirstAttribute.Value == key);
    if (node != null) { node.Remove(); }

    node = _data.Nodes().OfType<XElement>().FirstOrDefault(n => n.FirstAttribute.Value == key);
    if (node != null) { node.Remove(); }
}

private void AddNode(XElement element, string key, string value)
{
    if (key == null) { return; }

    XNamespace ns = "http://www.w3.org/XML/1998/namespace";

    var newValue = new XElement("data");
    newValue.SetAttributeValue("name", key);
    newValue.SetAttributeValue(ns + "space", "preserve");
    newValue.SetElementValue("value", value);

    element.Add(newValue);
}

public void Load()
{
    _window.Show();
}

private void LoadData()
{
    if (string.IsNullOrEmpty(DataPath) || string.IsNullOrEmpty(LocalizedDataPath))
    {
        return;
    }

    ResxStrings.Clear();

    _data = XElement.Load(DataPath);

    var dataStrings = _data.Nodes().OfType<XElement>().Where(n => n.LastAttribute.Name.LocalName == "space"
                                                               && n.LastAttribute.Value == "preserve"
                                                               && n.FirstAttribute.Name.LocalName == "name");

    _localizedData = XElement.Load(LocalizedDataPath);

    var localizedDataStrings = _localizedData.Nodes().OfType<XElement>().Where(n => n.LastAttribute.Name.LocalName == "space"
                                                                   && n.LastAttribute.Value == "preserve"
                                                                   && n.FirstAttribute.Name.LocalName == "name").ToList();

    // add all strings in data
    foreach (var node in dataStrings)
    {
        var localizedValue = localizedDataStrings.FirstOrDefault(n => n.FirstAttribute.Value == node.FirstAttribute.Value);

        ResxStrings.Add(new ResxValues(node.FirstAttribute.Value, node.Value.Trim(), localizedValue == null ? null : localizedValue.Value.Trim()));
    }

    // add all strings in localized data not already added
    foreach (var node in localizedDataStrings.Where(n => !ResxStrings.Select(s => s.Key).Contains(n.FirstAttribute.Value)))
    {
        ResxStrings.Add(new ResxValues(node.FirstAttribute.Value, null, node.Value.Trim()));
    }
}

public event PropertyChangedEventHandler PropertyChanged;
protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
{
    var handler = PropertyChanged;
    if (handler != null)
    {
        handler(this, new PropertyChangedEventArgs(propertyName));
    }
}

The App startup file:

protected override void OnStartup(StartupEventArgs e)
{
    var translationHelperVM = new ResxTranslationHelperVM(new ResxTranslationHelperWindow());
    translationHelperVM.Load();
}

The ResxValues class:

public string Key { get; set; }
public string Value { get; set; }
public string LocalizedValue { get; set; }

public ResxValues(string key, string value, string localizedValue)
{
    Key = key;
    Value = value;
    LocalizedValue = localizedValue;
}

public ResxValues() : this("", "", "") { }

And the RelayCommand class:

public event EventHandler CanExecuteChanged;
private readonly Action<object> _execute;

public RelayCommand(Action<object> execute)
{
    _execute = execute;
}

public bool CanExecute(object parameter)
{
    return true;
}

public void Execute(object parameter)
{
    _execute(parameter);
}

You can view (and use) the whole project on GitHub, and of course all comments are welcome.

Answer 1

I would like to add a few thoughts on WPF part of your code:

1. If you need your buttons to occupy half the screen, you should simply replace StackPanel with Grid:

   <Grid>
      <Grid.ColumnDefinitions>
          <ColumnDefinition/>
          <ColumnDefinition/>
      <Grid.ColumnDefinitions>
   </Grid>
   

   This is not the case where you should use a converter.

2. GridDisplay_LoadingRow - now this is a different story. Here converter will fit nicely. You can data bind your data grid item to Background property, and use a converter to set the appropriate Brush depending on item properties. You can use regular converter, or you can use a multibinding. The latter wil update background dynamically, if you implement INotifyPropertyChanged interface on your ResxValues class.

3. DataGrid_BeginningEdit - and what if user added a new item but noticed a typo afterwards? Why can't he edit it? He can remove the entire row and re-add it though. Thats a really weird design from UX standpoint. :) If you want to keep this logic, I would suggest using xaml and data binding instead. You can use DataGridTemplateColumn with custom CellEditingTemplate which will be a TextBox for editable cells or TextBlock for read only cells. You will select one or the other by binding to your data grid items. This might be a bit complicated if you've just started learning wpf though.

4. GridDisplay_CellEditEnding - so, if user makes a mistake you remove the entire row? :) That's not very user-friendly. I am not sure I understand, why you are recreating ItemsSource either, seems fishy to me (will it update the collection on your viewmodel? or will it break the binding?). WPF have an inbuild system for validating data errors. Normally you would want to let user know, that he made an error (show tooltip, highlight with red border, etc.) and give him a chance to fix it.

5. DataGrid_PreviewKeyDown - you should use InputBindings instead.

   <DataGrid.InputBindings>
       <KeyBinding Key="Delete" 
                   Command="{Binding DeleteCommandOnYourViewModel}" 
                   CommandParameter="{Binding ElementName=YourDataGridName, Path=SelectedItem}"/>
   </DataGrid.InputBindings>
   

If you choose to follow the above suggestions, your code behind file will look like this after the refactoring:

public ResxTranslationHelperWindow()
{
    InitializeComponent();
}

Answer 2

In the GridDisplay_LoadingRow() method, you should return after setting the Background of the current row. Otherwise, if e.g.item.LocalizedValue and item.Value are both == string.Empty you could receive a light flickering.

---

Be consistent. One time you are checking string.IsNullOrEmpty(), one time you use != "". If you are sure that a string isn't null you might just check variable.Length > 0 or != string.Empty. A "" check will lead to you rechecking is there really no space between (if you eyes getting worse).

---

In the DataGrid_BeginningEdit() method you have a magic number 0 which should be extracted to a const. Additionally, by using a guard clause and returning if the DisplayIndex doesn't hold the needed value (0) will make it more clear.

---

If it is possible in the GridDisplay_CellEditEnding() method that itemsLastRemoved is empty you should add a check regarding the Count property. Otherwise the catch will throw by calling RemoveAt() with a negative index.

And again, use a guard clause if (e.EditAction != DataGridEditAction.Commit) { return; }.

---

In the ResxTranslationHelperVM class , you should consider making the property setters private which don't need to be public accessible.

---

The EndCellEdit() method has more magic numbers in it. Also this

if (e.Row.Item as ResxValues == ResxStrings.Last() &&
    ResxStrings.Select(s => s.Key).Contains(key))
{
    MessageBox.Show(Resources.EndCellEdit_KeyAlreadyExists, Resources.EndCellEdit_AddValueError, MessageBoxButtons.OK,
               MessageBoxIcon.Warning);
    throw new ArgumentException("Invalid data");
}  

is not my favourite way of control flow. You are throwing an Argument for a specific case to be handled in the GridDisplay_CellEditEnding() method, which can also be handled in this EndCellEdit() method.

The condition ResxStrings.Select(s => s.Key).Contains(key) will first iterate over ResxStrings and select the Key property and that by calling Contains() on the result you are iterating over the "keys" until you find a matching one.

A more obvious and performant way would be to use the Any() method like ResxStrings.Any(s => s.Key == key).

---

The RemoveNodes() method is a little bit misleading, because it removes only one node. So naming the method RemoveNode() would be better.

You are checking here if node != null but you forgot to check if node.parent != null. If there could be the case that node.parent == null an InvalidOperationException would be thrown by calling Remove() on the node.

---

In the LoadData() method you have some duplicated code which could be extracted to a separate method. In addition one time you are using the result as IEnumerable<T> and the other time you are calling ToList() on the IEnumerable<T>.

I would suggest to extract this to

private const string xLastLocalName = "space";
private const string xValue = "preserve";
private const string xFirstLocalName = "name";
private IEnumerable<XElement> LoadElements(string filePath)
{
    var element = XElement.Load(filePath);
    return element.Nodes().OfType<XElement>().Where(n => n.LastAttribute.Name.LocalName == xLastLocalName 
                                                               && n.LastAttribute.Value == xValue 
                                                               && n.FirstAttribute.Name.LocalName == xFirstLocalName );

}

where with the const names I am not really satisfied.

This would reduce the calls in the LoadData() method to

var dataStrings = LoadElements(DataPath);
var localizedDataStrings = LoadElements(LocalizedDataPath);

If there is a possibility that the files doesn't exist you should handle this case too.

Answer 3

// add all strings in localized data not already added
foreach (var node in localizedDataStrings.Where(n => !ResxStrings.Select(s => s.Key).Contains(n.FirstAttribute.Value)))
{
    ResxStrings.Add(new ResxValues(node.FirstAttribute.Value, null, node.Value.Trim()));
}

That's an awful lot of horizontal scroll. I'd recommend moving the Linq outside the loop and adding some line breaks.

var matching = localizedDataStrings.Where(
    n => !ResxStrings.Select(s => s.Key)
                     .Contains(n.FirstAttribute.Value)
);

// add all strings in localized data not already added
foreach (var node in matching)
{
    ResxStrings.Add(new ResxValues(node.FirstAttribute.Value, null, node.Value.Trim()));
}

Of course, at this point, you may want to consider linq-ifying the whole kittenkaboodle.

Answer 4

This has a bug:

foreach (var node in localizedDataStrings.Where(n => !ResxStrings.Select(s => s.Key).Contains(n.FirstAttribute.Value)))
{
    ResxStrings.Add(new ResxValues(node.FirstAttribute.Value, null, node.Value.Trim()));
}

First, there is the obvious Trim(). That will remove any deliberately prefixed/suffixed spaces. The reason for that is I was getting the value wrong. When you get the value like this, it formats as \n {value}\n \n {comment}\n. The correct way of getting the value (and comment) is:

foreach (var node in localizedDataStrings.Where(n => !ResxStrings.Select(s => s.Key).Contains(n.FirstAttribute.Value)))
{
    ResxStrings.Add(new ResxValues(node.FirstAttribute.Value, null, node.Nodes().OfType<XElement>().FirstOrDefault(n => n.Name.LocalName == "value").Value));
}

To get the value of the comment, replace "value" with "comment".