WPF beyond my understanding



  • K, so, we have some interns, and they're quite good. We have them learning WPF and dealing with XML. I taught them basic LINQ and explained what an ItemsControl is for. Then they found something weird. They have code that works pretty much like this:

    private IEnumerable<InputType> defaults = new List<InputType>
    {
      new TextBoxInput("Blah", "Bleh"),
      new ComboBoxInput("Blah", users),
    }
    
    private IEnumerable<InputType> GetTheThings(XDocument doc)
    {
      var inputs = doc.GetInputElement(); // They do something better than this, I'm paraphrasing
      if(inputs.Any())
        thingOverThereViewModel.Inputs = inputs.Select(x => MakeAThing(x.Name, x.Element("Blah").Value, x.Element("Bleh").Value));
      else
        thingOverThereViewModel.Inputs = defaults;
    }
    
    private InputType MakeAThing(string name, string whatever, string thisToo)
    {
      if(whatever == "Bleh")
        return new ComboBoxInput(name, thisToo);
      return new TextBoxInput(name);
    }
    

    Now, the viewmodel's Inputs is bound to an itemscontrol, and when they cant find any things, the defaults work, and the values update. The actual things constructed get made, but a separate copy gets updated.

    I had them tack on a .ToList() after the .Select() thinking, "This will never work, and makes no sense, but let's try it."

    IT WORKED.

    WHY!?

    I get that LINQ is lazy, and I normally know how to deal with it. But this is bizarre!



  • I don't understand what's the problem.



  • Yeah, when in doubt, ToList() everything.



  • How is GetTheThings used?

    Is there some sort of temporal issue?



  • So, the only way you'd get this behavior that I can think of is:

    When you bind the ItemsSource to the IEnumerable, it iterates over each element, and then iterates again to assign values to the bindings. Whereas forcing it into a list means you just iterate over the same structure, rather than calling a constructor when you iterate.

    But it seems stupid beyond belief that they'd iterate twice.

    @Maciejasjmj said:

    Yeah, when in doubt, ToList() everything.

    Not without being sure it's a good idea. I mean, it's worth trying sure, but it's also massively overused, and causes performance issues if you spam it.



  • @Magus said:

    performance issues

    Well duh. But before you optimize for performance you generally want your shit to work.



  • Which is why I always carefully consider what code to write, rather than guessing '.ToList()'.



  • If it's a correct guess like 90 percent of the time (and make it 99 if you have Select around), I'd say it's a good first guess.



  • Sounds like good advice. My Windows troubleshooting rule of thumb is now:

    1. Reboot!
    2. .ToList()

    Good to know.



  • Ok, first of all, the code makes no sense because GetTheThings has no return or yield? Am I crazy?

    Secondly, List implements IList, which implements IEnumerable, so I'm not sure why you thought it wouldn't work. All you're doing is defeating the lazy evaluation by telling LINQ to evaluate it right there right then.



  • My guess is "because he thought it doesn't matter since whatever is bound to Inputs should enumerate the IEnumerable anyway".



  • Because each time you iterate over the select, you're generating a new list. Because the anonymous method is returning new objects each time.

    Items is not a real list, it's an IEnumerator, so every time you do a foreach on it, you get a new list of items.

    You're iterating twice likely because the first iteration is to establish the entities the items control is building, the second iteration is to apply the datacontexts to the child views.



  • But even if I'm wrong in the previous post, it doesn't matter.

    @Magus said:

    Not without being sure it's a good idea.

    No, what you avoid is you don't return new for a select, and if you do, you evaluate it before you store it somewhere.

    If you only want to initialize once, you collapse the select when you want it initialized.

    Don't ever assume how an external party is going to behave with your IEnumerable.



  • @xaade said:

    You're iterating twice likely because the first iteration is to establish the entities the items control is building, the second iteration is to apply the datacontexts to the child views.

    That's exactly the conclusion I came to. And it makes no sense. I would have expected them to build a list or array or something, because having an accidental generator isn't all that hard, and I can't think of when binding to a generator would not be a bug. There should be something telling you what's wrong at the very least.

    @blakeyrat said:

    Ok, first of all, the code makes no sense because GetTheThings has no return or yield? Am I crazy?

    I wrote this all just a bit ago. It's essentially pseudocode to demonstrate approximately what the situation was.

    @blakeyrat said:

    I'm not sure why you thought it wouldn't work.

    At the time, I hadn't come to terms with the fact that it's a generator.



  • @Magus said:

    I would have expected them to build a list or array or something

    I don't see why they'd want to wrap the list you provided, in a list, just for the sake of iteration.

    The view isn't going to directly create or delete view objects, it's going to call the viewmodel to do that, because it has no way to update the original list.

    So the safest thing to do is just use the list it has been given.

    Plus, if you wrap an Observable collection, you lose the notifications, which means you lose the datacontext binding for that list. Unless you're going to wrap it in another list that forwards those events.

    In the end, there's no advantage for WPF to wrap in its own list.

    The UI objects will have their own list, but they are going to reference the data context for updates every single time.

    Once the UI objects exist, they can bind directly to the child object, but until that point they'll have to iterate the original list.

    EDIT:

    I'm pretty sure WPF and databinding came before yield return was used for LINQ.
    So, it's likely they never needed to consider this possibility.



  • @xaade said:

    Plus, if you wrap an Observable collection, you lose the notifications, which means you lose the datacontext binding for that list. Unless you're going to wrap it in another list that forwards those events.

    Now, see, in all of my code, I bind to List or ObservableCollection in almost every case. Even IEnumerable, sometimes. But this is some interns trying to parse XML into UI elements. It isn't that strange to see a generator this close to the xml, and elements aren't going to be added or removed. Really, the whole object should have been a list, and that would have made things simpler, but it isn't even slightly intuitive that this would happen.

    @xaade said:

    I'm pretty sure WPF and databinding came before yield return was used for LINQ.So, it's likely they never needed to consider this possibility.

    Now this makes sense and is probably exactly the reason for this. All I know is, I'd like something to tell me if something like this is happening. Maybe I can write a code analysis reference thingy. Those exist now...



  • @Magus said:

    I bind to List or ObservableCollection in almost every case. Even IEnumerable, sometimes. But this is some interns trying to parse XML into UI elements. It isn't that strange to see a generator this close to the xml, and elements aren't going to be added or removed.

    Solved problem.
    ObservableCollection<T> Constructor (IEnumerable<T>)



  • Why are you linking me the reference for a class you bind to when you need to add/remove elements? That isn't useful at all if the goal is to set a list of elements once. ToList works for that, and it's solved already. But what I want is for something to show a warning if you make a binding to an IEnumerable that acts as a generator. And I have no doubt I can do that, since it's built into 2015.



  • Oh, I missed that the XML document was read-only.



  • It's basically, read some XML, generate a UI to fill in some values, write some XML, which generates TFS work items. It's a good project for interns imo.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.