Posted by: Steve Walmsley
« on: Today at 05:26:18 AM »I don't want to turn this thread into a programming debate, which is one of the reasons that I rarely post code
I hope that none of the other readers of this thread have been put out by our monopolization of the topic of discussionAre you suggesting that instead of a new list, I use the LINQ without assigning to a new list as the target in the foreach loop?
If so, I prefer to avoid that. It's harder to read and harder to debug and those are more important to me than straight performance.
Yes, both the CurrentSystems and PossibleSystems variables would be IEnumerable<KnownSystem> instead of List<KnownSystem>. I can’t really see how the code would be more difficult to read, since everything else would be identical except that you would just not call ToList().
However, I suppose I know what you mean about debugging. You want to stop at a breakpoint and be able to view the lists in the debugger. I tend to forget that not everybody can use omniscient debugging yet (I use a debugger called Pernosco a lot, but it is for Linux only.) If I were using a regular debugger and I wanted to verify which systems had been in the final enumeration, I would add a conditional breakpoint inside the loop to print out some information about ks (the system name and number, for instance) and then continue execution. (In Pernosco you can do the same thing but you get all the information that would have been printed without having to actually run the program again.)
As for performance, if this code is just called once any time a fleet transits an unexplored jump point then it is probably irrelevant. Even though you make two whole copies of the list of systems (and you did just make that list significantly larger), that probably takes so little time that nobody will notice. But you might take a look to see if you have made this mistake in more important parts of the code. What is a very minor mistake here would be much more regrettable in the code that implements ship detection, for instance.
OK, I think I finally understand what you are driving at
I was concerned I would have the LINQ in the middle of the foreach statement, if I didn't assign it to a List() first and use that as the target for the foreach. Are you saying I could just replace the List<KnownSystem> = with IEnumerable<KnownSystem> = , remove the ToList() and then use the IEnumerable as the target of the foreach. That would be the same in terms of readability and I assume I could check the contents of the IEnumerable instead of the List?
I basically picked up C# by writing it and figuring things out when I needed to, without any formal learning, so there will be gaps in my knowledge
So now you have made me curious, I started reading about it. Some people expressed a view that IEnumerable only guarantees an enumeration and doesn't guarantee the order of execution (even if ordered beforehand). However, as I read it, its the same execution as list because the latter inherited the former. Which is correct?
EDIT
I just ran an experiment for the missile salvo list on the fleet window, which I just updated.
This is the ToList() version.
List<MissileSalvo> Salvos = Aurora.MissileSalvos.Values.Where(x => x.SalvoRace == this).OrderBy(x => x.SalvoSystem.SystemID).ThenBy(x => x.MissileSalvoID).ToList();
This is looped and various information is used to display missile salvos, using foreach (MissileSalvo ms in Salvos)
I then created a IEnumerable version of the same function
IEnumerable<MissileSalvo> Salvos = Aurora.MissileSalvos.Values.Where(x => x.SalvoRace == this).OrderBy(x => x.SalvoSystem.SystemID).ThenBy(x => x.MissileSalvoID);
I ran both functions several times and in every case, the List version is faster than the IEnumerable version. It may be that the IEnumerable is faster to create, but the List seems to be faster to use. Maybe because everything is preloaded into memory for the List, whereas the IEnumerable has to retrieve the information as needed.
