CODEWITHSUNDEEP
c#
Sean Anderson
Sean Anderson

Reputation: 29251

Better way to write this code rather than 2x foreach?

    List<CormantRadPane> panesToSave = new List<CormantRadPane>();

    foreach (KeyValuePair<string, RadPaneSetting> paneState in RadControlStates.PaneStates)
    {
        CormantRadPane pane = Utilities.FindControlRecursive(Page, paneState.Key) as CormantRadPane;
        panesToSave.Add(pane);
    }

    foreach (CormantRadPane pane in panesToSave)
    {
        RadControlSave.SavePane(pane);
    }

RadControlSave.SavePane(pane) modifies the RadControlStates.PaneStates collection. Is there a nicer way to write this code in this situation?

EDIT: Everyone please read what I wrote literally right above this. Jimmy posted an obvious solution that I hadn't though about -- creating a copy of the collection right before iterating through it so that RadControlSave does not modify the collection being iterated upon.

Upvotes: 1

Views: 189

Answers (9)

null_pointer
null_pointer

Reputation: 1849

How about this

foreach (var pane in
                RadControlStates.PaneStates
                .Select(paneState => Utilities
                    .FindControlRecursive(Page, paneState.Key) as CormantRadPane))
                RadControlSave.SavePane(pane);

Upvotes: 0

Justin Niessner
Justin Niessner

Reputation: 245399

Since calling RadControlSave.SavePane() modifies the collection, there's really no better way to do this.

Considering there's nothing fundamentally wrong with this code, I wouldn't worry about it too much. You could use LINQ, though, to make it look a little cleaner (even though it would be doing the same fundamental thing underneath):

var panesToSave = RadControlStates.PaneStates
    .Select(ps => Utilities.FindControlRecursive(Page, ps.Key) as CormantRadPane)
    .ToList();

foreach(var pane in panesToSave)
{
    RadControlSave.SavePane(pane);
}

I'm not familiar with any of those static methods though. If there's a way you could rewrite them to not modify the state of the collection...that would be preferable (it would allow you to do all the work in a single loop).

Update

You could also clone the original collection and combine the work in a single loop:

foreach(var kvp in RadControlStates.PaneStates.ToList())
{
    RadControlSave.SavePane(
        Utilities.FindControlRecursive(Page, paneState.Key) as CormantRadPane
    );
}

Upvotes: 0

Eamonn McEvoy
Eamonn McEvoy

Reputation: 8986

Is the panesToSave List neccessary why not simply have:

  foreach (KeyValuePair<string, RadPaneSetting> paneState in RadControlStates.PaneStates)     
    {         
    CormantRadPane pane = Utilities.FindControlRecursive(Page, paneState.Key) as CormantRadPane;         
    RadControlSave.SavePane(pane);    
    }

Hope This Helps, Eamonn

Upvotes: 0

Jimmy
Jimmy

Reputation: 91432

one way would be to call ToArray() on PaneStates (to make a copy, because you can't modify the sequence inside the foreach)

foreach(var state in RadControlStates.PaneStates.ToArray()) {
    var pane = Utilities.FindControlRecursive(Page, state.Key) as CormantRadPane;
    RadControlSave.SavePane(pane);
}

Upvotes: 5

taylonr
taylonr

Reputation: 10790

Do you have a collection of the Panes you're trying to save (instead of FindControlRecursive?)

Then you could use LINQ on that collection like:

foreach(var pane in Panes.Where(p => p.IsPane))
{
    RadControlSave.SavePane(pane);
}

Upvotes: 0

MUG4N
MUG4N

Reputation: 19717

How about this:

  List<CormantRadPane> panesToSave = new List<CormantRadPane>();



  foreach (KeyValuePair<string, RadPaneSetting> paneState in RadControlStates.PaneStates)
  {
        CormantRadPane pane = Utilities.FindControlRecursive(Page, paneState.Key) as CormantRadPane;
        //panesToSave.Add(pane);
        RadControlSave.SavePane(pane);
  }

Upvotes: 0

Carlos Valenzuela
Carlos Valenzuela

Reputation: 834

I think you could, instead of add it (in the first foreach) just save it

Upvotes: 0

skaz
skaz

Reputation: 22580

Can't you just combine them?

List<CormantRadPane> panesToSave = new List<CormantRadPane>();

foreach (KeyValuePair<string, RadPaneSetting> paneState in RadControlStates.PaneStates)
{
    CormantRadPane pane = Utilities.FindControlRecursive(Page, paneState.Key) as CormantRadPane;
    RadControlSave.SavePane(pane);
    panesToSave.Add(pane);
}

Upvotes: 0

squillman
squillman

Reputation: 13641

Can't you just do the SavePane() in the first foreach?

foreach (KeyValuePair<string, RadPaneSetting> paneState in RadControlStates.PaneStates)
{
    CormantRadPane pane = Utilities.FindControlRecursive(Page, paneState.Key) as CormantRadPane;
    RadControlSave.SavePane(pane);
}

Upvotes: 0

Related Questions