3
\$\begingroup\$

I feel like I had to invent a whole new way to do this because I couldn't find anything that did this the way I wanted to...

The basic process involves reading changes to cells and capturing information about those changes as they relate to the actual Entities in question and then persisting them to the database.

Here's the XAML. I didn't use databinding because I don't quite understand it yet and I'm on a bit of a deadline for this one so that's item number 1 on my to do list when I get a break.

Note that this project has been my first every foray into WPF. I've dabbled a bit in the past but I've really pushed myself to do things properly here... As properly as I know how anyway.

<DataGrid Name="dgUsers" Height="150"
    CellEditEnding="dgUsers_CellEditEnding">
</DataGrid>

<StackPanel Name="spActions" Background="#2d2d30" Width="500" Height="50" Orientation="Horizontal" FlowDirection="RightToLeft">
    <Button Name="btnCancel" Click="btnClose_Click" Content="Cancel" Style="{StaticResource CancelButton}" Height="30" Width="66" Margin="5,3,5,5"></Button>
    <Button Name="btnSave" Click="btnSave_Click" Content="Save" Style="{StaticResource RoundedButtonGreen}" Height="30" Width="66" Margin="5,3,5,5"></Button>
    <Label Name="lblResult"></Label>
</StackPanel>

Here's the C# codebehind:

Generate an entity from the changes that were made:

private List<User> UpdatedUsers { get; set; }
/// <summary>
/// Maintains a list of all changes made to all entities on the datagrid
/// </summary>
/// <param name="sender"></param>
/// <param name="e"></param>
private void dgUsers_CellEditEnding(object sender, DataGridCellEditEndingEventArgs e)
{
    if (UpdatedUsers == null) UpdatedUsers= new List<User>();

    var _User = (User)e.Row.Item;
    var Element = (TextBox)e.EditingElement;
    if (String.Equals((string)e.Column.Header, "EmailAddress"))
    {
        User.EmailAddress = Element.Text;
    }
    else if (String.Equals((string)e.Column.Header, "Password"))
    {
        User.Password = Element.Text;
    }

    UpdatedUsers.Add(User);
}

From here the user clicks on a save button which loops through the changes captured in this list and updates the relevant records in the database:

private void btnSave_Click(object sender, RoutedEventArgs e)
{
    try
    {
        ApplicationDbContext Context = new ApplicationDbContext();
        var Users = Context.Users.Where(x => x.CompanyId == (int)cmbCompanies.SelectedValue).ToList();

        foreach (User User in Users)
        {
            foreach (User U in UpdatedUsers)
            {
                if (U.Id == User.Id)
                {
                    User.EmailAddress = U.EmailAddress;
                    User.Password = U.Password;
                }
                Context.SaveChanges();
            }
        }
    }
    catch (Exception ex)
    {
        lblResult.Content = ex.Message;
    }       
}
\$\endgroup\$
5
  • \$\begingroup\$ The xaml doesn't use databinding, no. I've gone through tutorials but I don't understand how that works just yet so rather than use up the project development time I decided I'd quickly whip this together \$\endgroup\$
    – Ortund
    Jun 5, 2017 at 8:44
  • \$\begingroup\$ Didn't even take me half an hour to do this though. I'll definitely replace this with databinding in due time but for now deadline is fast approaching so I figured I'd stick to something that I knew would work without much head scratching \$\endgroup\$
    – Ortund
    Jun 5, 2017 at 8:50
  • 1
    \$\begingroup\$ @Ortund I'm not sure what kind of review you are expecting to get then. "This code violates everything WPF stands for" is probably not very helpful, but that's how it looks. And using proper naming conventions won't change that. You need to get at least basic understanding of MVVM before using WPF in production code. Deadline is a poor excuse, IMHO: data binding is not rocket science, it's pretty basic stuff, that can be quickly explained/learned if you already have a basic knowledge of WPF (e.g. see codeproject.com/Articles/165368/WPF-MVVM-Quick-Start-Tutorial). \$\endgroup\$
    – Nikita B
    Jun 5, 2017 at 9:26
  • \$\begingroup\$ So rather than critiquing the code on the basis that it isn't "what WPF stands for", how about evaluating it on its merits? I know that's a lot to ask in this world but what do you say we actually make some sort of effort for a change? \$\endgroup\$
    – Ortund
    Jun 5, 2017 at 9:50
  • \$\begingroup\$ @Ortund, I used some harsh wording and I'm sorry if I offended you. It was not my intention. \$\endgroup\$
    – Nikita B
    Jun 5, 2017 at 10:48

1 Answer 1

4
\$\begingroup\$

Other issues...

  • C# variable naming convention is camelCase. And definitely don't start a local's name with an underscore, as underscores are understood to prefix instance variables.
  • There's no separation of concerns here. Your UI code is talking to the database. That's not the responsibility of the UI.
  • I assume ApplicationDbContext inherits from DbContext which is implements IDisposable. You're not disposing it. Use it in a using block. (I know nothing about Entity Framework so don't know if there's a good reason not to do so.)
\$\endgroup\$
2
  • \$\begingroup\$ Well thank you :) for being the first genuinely helpful person who's participated on this post. I was sure I read something about conventions on MSDN that supported the way I do it (perhaps I was wrong) but you're absolutely right that I should be using the context in a using block. \$\endgroup\$
    – Ortund
    Jun 5, 2017 at 20:51
  • \$\begingroup\$ @Ortund I can't find the link I had to the naming conventions; current MSDN guidance doesn't appear to say anything about private fields and variables, but check out the .NET section in the document here 1code.codeplex.com/downloads/get/357518?releaseId=84683 where it is covered. \$\endgroup\$
    – 404
    Jun 5, 2017 at 22:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.