Want to remove the goto in my code

Question

I want to remove my goto's in this code but I'm not sure how. There must be a cleaner way to write my code. Perhaps move the "if" statement to the end of each loop then use break?

Sorry if this is a stupid question, I'm self-taught, and I'm finishing up my first sizable program. Any suggestions as to how I can clean my code is welcomed, even though it isn't stated in my question.

Also this is just my beta release, so please forgive all the sillyness.

    public void BtnRandomizeClick(object sender, EventArgs e)
    {
    Nowthatitssaved:   //Goto marker -- Loops the saved/not saved stuff based on file dialog
        switch (MessageBox.Show(@"Did you SAVE your edited list?", @"Just checking... It's important.",
                                MessageBoxButtons.YesNo))
        {
                #region Dialog Yes

            case DialogResult.Yes:
                if (FileChosen == null)
                {
                    MessageBox.Show(
                        @"Please select a file to load before trying to randomize. You need some entries to randomize first eh?",
                        "Oh fiddlesticks...");
                    return;
                }
                dataGridAssociates.Rows.Clear();
                var masterList = new List<Associate>();
                var divertsList = new List<Associate>();
                var mheList = new List<Associate>();
                var loadingList = new List<Associate>();
                var lines = File.ReadAllLines(FileChosen); //Array of all the lines in the text file

                foreach (var assocStringer in lines)
                {
                    if (assocStringer == null) continue;
                    var entries = assocStringer.Split('|');
                    var obj = (Associate) _bindingSource.AddNew();
                    if (obj == null) continue;
                    if (entries.Length < 7) break;
                    obj.FirstName = entries[0];
                    obj.LastName = entries[1];
                    obj.AssocId = entries[2];
                    obj.AssocRfid = entries[3];
                    obj.CanDoDiverts = Boolean.Parse(entries[4]);
                    obj.CanDoMhe = Boolean.Parse(entries[5]);
                    obj.CanDoLoading = Boolean.Parse(entries[6]);
                    obj.UniqueRandomID = Guid.NewGuid().ToString();
                    masterList.Add(obj);
                }
                var sortedMaster = masterList.OrderBy(d => d.UniqueRandomID).ToList();
                var loaderCountRequested = 0;
                var mheCountRequested = 0;
                var divertCountRequested = 0;
                var validDivertEntry = int.TryParse(txtDivertWorkers.Text, out divertCountRequested);
                var validMHEEntry = int.TryParse(txtMHEWorkers.Text, out mheCountRequested);
                var validLoaderEntry = int.TryParse(txtLoadersNeeded.Text, out loaderCountRequested);

                //See if whomever used the inputs correctly
                if (validDivertEntry == false)
                {
                    MessageBox.Show(@"Please use an integer in the 'Divert Workers' textbox and try again");
                    return;
                }
                if (validMHEEntry == false)
                {
                    MessageBox.Show(@"Please use an integer in the 'MHE Workers' textbox and try again");
                    return;
                }
                if (validLoaderEntry == false)
                {
                    MessageBox.Show(@"Please use an integer in the 'Loaders' textbox and try again");
                    return;
                }

                var canDoDivertsQuery = sortedMaster.Where(a => a.CanDoDiverts);
                var canDoMHEQuery = sortedMaster.Where(b => b.CanDoMhe);
                var canDoLoadingQuery = sortedMaster.Where(c => c.CanDoLoading);
                var loaders = 0;
                var mhes = 0;
                var diverters = 0;

                //Begin the sorting method
                foreach (Associate someAssocVar in canDoLoadingQuery)
                {
                    if (loaders == loaderCountRequested)
                        goto MHEKTHX;
                    loadingList.Add(someAssocVar);
                    loaders++;
                    listBoxLoaders.Items.Add(loadingList[loaders - 1].FirstName + " " +
                                             loadingList[loaders - 1].LastName);

                }
           MHEKTHX:
                sortedMaster.RemoveAll(loadingList.Contains);

            foreach (Associate someAssocVar in canDoMHEQuery)
                {
                    if (mhes == mheCountRequested)
                        goto DivertsKTHX;
                    mheList.Add(someAssocVar);
                    mhes++;
                    listBoxMHE.Items.Add(mheList[mhes - 1].FirstName + " " +
                                         mheList[mhes - 1].LastName);
                }

            DivertsKTHX:
                sortedMaster.RemoveAll(mheList.Contains);

                foreach (Associate someAssocVar in canDoDivertsQuery)
                {
                    if (diverters == divertCountRequested)
                        goto DoneKTHX;
                    divertsList.Add(someAssocVar);
                    diverters++;
                    listBoxDiverts.Items.Add(divertsList[diverters - 1].FirstName + " " +
                                             divertsList[diverters - 1].LastName);
                }

           DoneKTHX:
                sortedMaster.RemoveAll(divertsList.Contains);

                MessageBox.Show(@"You have " + sortedMaster.Count() + @" associates unassigned.");
                break;
                #endregion

                #region Dialog No

            case DialogResult.No:
                if (MessageBox.Show(@"Would you like to save your work then?", @"It really does matter...",
                                    MessageBoxButtons.YesNo) == DialogResult.Yes)
                {
                    var saveDialog = new SaveFileDialog
                                         {
                                             InitialDirectory =
                                                 Convert.ToString(Environment.SpecialFolder.MyDocuments),
                                             // ReSharper disable LocalizableElement
                                             Filter = "Text (*.txt)|*.txt|All Files (*.*)|*.*",
                                             // ReSharper restore LocalizableElement
                                             FilterIndex = 1
                                         };
                    if (saveDialog.ShowDialog() == DialogResult.OK)
                    {
                        FileChosen = saveDialog.FileName;
                        var assocToText = DatagridToString(dataGridAssociates, '|');
                        using (var outfile = new StreamWriter(saveDialog.FileName))
                            outfile.Write(assocToText);
                    }
                    goto Nowthatitssaved;
                }
                break;

                #endregion
        }
    }

Answer 1

Here's a variety of refactorings. I didn't start off intending to change the code this much, but I figure I might as well post it now...

You say "I think using methods is something I'm missing from my whole program...", and I agree. Extracting out code into methods is the single best thing you can do to improve this. You will make your code more structured and easier to understand, and be able to remove any redundancies.

Now some individual items:

• Firstly, the Random ID on the Associate class appears to only be needed to shuffle the list. It is not an intrinsic part of the Associate, so it doesn't belong there. Remove it and shuffle the list directly.

• One thing that often helps is to always declare your variables as late as possible. This tends to reveal the structure of your code better. Let's do that with the sorting part of the code:

  var loadingList = new List<Associate>();
  var canDoLoadingQuery = masterList.Where(c => c.CanDoLoading);
  var loaders = 0;
  foreach (Associate someAssocVar in canDoLoadingQuery)
  {
      if (loaders == loaderCountRequested)
          break;
      loadingList.Add(someAssocVar);
      loaders++;
      listBoxLoaders.Items.Add(loadingList[loaders - 1].FirstName + " " +
                                  loadingList[loaders - 1].LastName);
  
  }
  masterList.RemoveAll(loadingList.Contains);
  
  // one removed to save space
  
  var divertsList = new List<Associate>();
  var canDoDivertsQuery = masterList.Where(a => a.CanDoDiverts);
  var diverters = 0;
  foreach (Associate someAssocVar in canDoDivertsQuery)
  {
      if (diverters == divertCountRequested)
          break;
      divertsList.Add(someAssocVar);
      diverters++;
      listBoxDiverts.Items.Add(divertsList[diverters - 1].FirstName + " " +
                                  divertsList[diverters - 1].LastName);
  }
  masterList.RemoveAll(divertsList.Contains);
  

Notice that they all have the same structure. This should indicate to you that this is common code that can be shared. Let's extract one instance into a method:

private List<Associate> AssignLoaders(List<Associate> masterList, int loaderCountRequested)
{
    var loadingList = new List<Associate>();
    var canDoLoadingQuery = masterList.Where(c => c.CanDoLoading);
    var loaders = 0;
    foreach (Associate someAssocVar in canDoLoadingQuery)
    {
        if (loaders == loaderCountRequested)
            break;
        loadingList.Add(someAssocVar);
        loaders++;
        listBoxLoaders.Items.Add(loadingList[loaders - 1].FirstName + " " +
                                    loadingList[loaders - 1].LastName);
    }
    return loadingList;
}

This is called like so:

var loadingList = AssignLoaders(masterList, loaderCountRequested);
masterList.RemoveAll(loadingList.Contains);

Now how can we make this more generic so that we can share it with the other two uses? We need to be able to pass in the condition. With LINQ we can use a Func<Associate, bool> - that is, a function which takes an associate and returns a bool. You already have some of these in your code (the c => c.CanDoLoading in the Where calls).

Let's make the condition a parameter:

private List<Associate> AssignLoaders(List<Associate> masterList, Func<Associate, bool> filter, int loaderCountRequested)
{
    var loadingList = new List<Associate>();
    var canDoLoadingQuery = masterList.Where(filter);
    var loaders = 0;
    foreach (Associate someAssocVar in canDoLoadingQuery)
    {
        if (loaders == loaderCountRequested)
            break;
        loadingList.Add(someAssocVar);
        loaders++;
        listBoxLoaders.Items.Add(loadingList[loaders - 1].FirstName + " " +
                                    loadingList[loaders - 1].LastName);
    }
    return loadingList;
}

This is called like this:

var loadingList = AssignLoaders(masterList, c => c.CanDoLoading, loaderCountRequested);

Now the AssignLoaders knows almost nothing about whether it is looking for loaders, so we can use it for the other types as well. Let's rename the variables to indicate that, then tidy it up (with a little LINQ) and make it able to assign to any destination listbox:

private List<Associate> AssignAssociates(IEnumerable<Associate> associates, Func<Associate, bool> filter, int numberRequested, ListBox destination)
{
    var assignedAssociates = new List<Associate>();
    assignedAssociates.AddRange(associates.Where(filter).Take(numberRequested));
    foreach (var associate in assignedAssociates)
    {
        destination.Items.Add(associate.FirstName + " " + associate.LastName);
    }
    return assignedAssociates;
}

The listbox stuff doesn't really fit right here, it looks a bit ugly. Let's split it into two separate methods:

private void AddAssociatesToListBox(List<Associate> associates, ListBox listBox)
{
    foreach (var associate in associates)
    {
        listBox.Items.Add(associate.FirstName + " " + associate.LastName);
    }
}

private List<Associate> AssignAssociates(IEnumerable<Associate> associates, Func<Associate, bool> filter, int numberRequested)
{
    return associates.Where(filter).Take(numberRequested).ToList();
}

Now the sorting code looks like:

var loadingList = AssignAssociates(masterList, c => c.CanDoLoading, loaderCountRequested);
AddAssociatesToListBox(loadingList, listBoxLoaders);
masterList.RemoveAll(loadingList.Contains);

// one skipped to save space

var divertsList = AssignAssociates(masterList, c => c.CanDoDiverts, divertCountRequested);
AddAssociatesToListBox(divertsList, listBoxDiverts);
masterList.RemoveAll(divertsList.Contains);

Additionally, if you give the Associate class a ToString override then AddAssociatesToListBox becomes irrelevant (delete it!) and you can just write:

var divertsList = AssignAssociates(masterList, c => c.CanDoDiverts, divertCountRequested);
listBoxDiverts.DataSource = divertsList;
masterList.RemoveAll(divertsList.Contains);

• We can do a similar refactoring to this previous one with the validation code:

  int loaderCountRequested;
  if (!CountIsValid(txtLoadersNeeded, "Loaders", out loaderCountRequested)) return;
  

And the extracted method:

private bool CountIsValid(TextBox textBox, string textBoxName, out int countRequested)
{
    var validEntry = int.TryParse(textBox.Text, out countRequested);
    if (validEntry == false)
    {
        MessageBox.Show(string.Format(@"Please use an integer in the '{0}' textbox and try again.", textBoxName));
    }
    return validEntry;
}

I'd actually recommend you look at using one of the built-in validation mechanisms, but I'm not very well versed in Windows Forms. Maybe someone can point the way in a comment.

• For your main method, I don't think you need to reload the file each time. It makes sense to randomize what is currently in the data grid. As long as you don't clear it your user won't lose any data.

This actually means that your complicated goto logic simply disappears as it is not needed. (You will see this in the code I post below.)

• Separate out the loading and saving code. This makes it a bit cleaner and easier to see what is going on, and allows us to do other things more easily, like:

• Track the state of the data. By checking if the user has modified the data, you can be smarter about asking them when to save their changes. This can be done by attaching to an event on the grid. I've called the variable that tracks this _unsavedChanges. It is also reset when the user loads or saves data.

Here are my modifications in toto. I've guessed at the implementation of DatagridToString but it seems to work:

public partial class Main : Form
{
    public Main()
    {
        InitializeComponent();

        _unsavedChanges = false;
    }

    private string _currentFile;

First, the randomization method and its minions:

    void BtnRandomizeClick(object sender, EventArgs e)
    {
        if (_currentFile == null)
        {
            MessageBox.Show(@"Please select a file to load before trying to randomize. You need some entries to randomize first eh?",
                "Oh fiddlesticks...");
            return;
        }

        // Get a list of all associates
        var masterList = _bindingSource.OfType<Associate>().ToList();
        masterList.Shuffle(new Random());

        //See if whomever used the inputs correctly
        int divertCountRequested;
        if (!CountIsValid(txtDivertWorkers, "Divert Workers", out divertCountRequested)) return;

        int mheCountRequested;
        if (!CountIsValid(txtMHEWorkers, "MHE Workers", out mheCountRequested)) return;

        int loaderCountRequested;
        if (!CountIsValid(txtLoadersNeeded, "Loaders", out loaderCountRequested)) return;

        //Begin the sorting method
        var loadingList = AssignAssociates(masterList, c => c.CanDoLoading, loaderCountRequested);
        listBoxLoaders.DataSource = loadingList;
        masterList.RemoveAll(loadingList.Contains);

        var mheList = AssignAssociates(masterList, c => c.CanDoMhe, mheCountRequested);
        listBoxMHE.DataSource = mheList;
        masterList.RemoveAll(loadingList.Contains);

        var divertsList = AssignAssociates(masterList, c => c.CanDoDiverts, divertCountRequested);
        listBoxDiverts.DataSource = divertsList;
        masterList.RemoveAll(divertsList.Contains);

        MessageBox.Show(@"You have " + masterList.Count() + @" associates unassigned.");
    }

    private bool CountIsValid(TextBox textBox, string textBoxName, out int countRequested)
    {
        var validEntry = int.TryParse(textBox.Text, out countRequested);
        if (validEntry == false)
        {
            MessageBox.Show(string.Format(@"Please use an integer in the '{0}' textbox and try again.", textBoxName));
        }
        return validEntry;
    }

    private List<Associate> AssignAssociates(IEnumerable<Associate> associates, Func<Associate, bool> filter, int numberRequested)
    {
        return associates.Where(filter).Take(numberRequested).ToList();
    }

Now, code that loads and saves the files:

    private bool _unsavedChanges;

    private bool CheckForUnsavedChanges()
    {
        if (_unsavedChanges)
        {
            if (MessageBox.Show(@"Would you like to SAVE your edited list?", @"Save your changes?", MessageBoxButtons.YesNo) == DialogResult.Yes)
            {
                if (!PromptToSaveFile())
                    return false; // the user cancelled
            }
        }

        return true;
    }

    private void LoadFile(string filename)
    {
        _bindingSource.Clear(); // remove any old entries

        var lines = File.ReadAllLines(filename); //Array of all the lines in the text file
        foreach (var assocStringer in lines)
        {
            if (assocStringer == null) continue;
            var entries = assocStringer.Split('|');
            var obj = (Associate) _bindingSource.AddNew();
            if (obj == null) continue;
            if (entries.Length < 7) break;
            obj.FirstName = entries[0];
            obj.LastName = entries[1];
            obj.AssocId = entries[2];
            obj.AssocRfid = entries[3];
            obj.CanDoDiverts = Boolean.Parse(entries[4]);
            obj.CanDoMhe = Boolean.Parse(entries[5]);
            obj.CanDoLoading = Boolean.Parse(entries[6]);
        }
        _unsavedChanges = false;
    }

    string DatagridToString(DataGridView dataGrid, char joiner)
    {
        var join = joiner.ToString();
        var sb = new StringBuilder();
        foreach (DataGridViewRow row in dataGrid.Rows)
        {
            var cells = row.Cells.Cast<DataGridViewCell>();
            if (cells.Any(cell => cell.Value == null)) // for now, skip any lines with null cells
                continue;

            sb.AppendLine(string.Join(join, cells.Select(cell => cell.Value))); // concatenate the cells
        }
        return sb.ToString();
    }

    bool PromptToOpenFile()
    {
        var openDialog = new OpenFileDialog()
        {
            InitialDirectory = Convert.ToString(Environment.SpecialFolder.MyDocuments),
            Filter = @"Text (*.txt)|*.txt|All Files (*.*)|*.*",
            FilterIndex = 1
        };

        if (openDialog.ShowDialog() == DialogResult.OK)
        {
            _currentFile = openDialog.FileName;
            LoadFile(_currentFile);
            return true;
        }

        return false;
    }

    bool PromptToSaveFile()
    {
        var saveDialog = new SaveFileDialog
        {
            InitialDirectory = Convert.ToString(Environment.SpecialFolder.MyDocuments),
            Filter = @"Text (*.txt)|*.txt|All Files (*.*)|*.*",
            FilterIndex = 1,
            CheckFileExists = true,
        };

        if (saveDialog.ShowDialog() == DialogResult.OK)
        {
            _currentFile = saveDialog.FileName;
            SaveFile(_currentFile);
            return true;
        }

        return false;
    }

    void SaveFile(string filename)
    {
        var assocToText = DatagridToString(dataGridAssociates, '|');
        using (var outfile = new StreamWriter(filename))
            outfile.Write(assocToText);

        _unsavedChanges = false;
    }

And finally the events. You should be able to figure out what they do by their names. (There are two more buttons, Save and Load.)

    private void btnSave_Click(object sender, EventArgs e)
    {
        PromptToSaveFile();
    }

    private void btnLoad_Click(object sender, EventArgs e)
    {
        if (!CheckForUnsavedChanges())
            return;

        PromptToOpenFile();
    }

    private void dataGridAssociates_CellValueChanged(object sender, DataGridViewCellEventArgs e)
    {
        _unsavedChanges = true;
    }
}

I've uploaded my whole project here. It should be runnable as-is. It contains the shuffle method, my Associate object, and the form.

Answer 2

Here's another stab at something. Mainly I think I see a bunch of duplicated code in your lists, so what about something along the lines of

private IList<Associate> GetAssociates(IEnumerable<Associate> query,  int loadFrom, int maxItems)
{
    var numberRequested = maxItems - loadFrom;
    return query.Take(numberRequested).ToList();
}

private void LoadAssociatesInto(ListBox container, IEnumerable<Associate> associates)
{
    container.Items.AddRange(associates.Select(p => string.format("{0} {1}", p.FirstName, p.LastName)).ToArray<object>());
}

// ... and your main code
var associates = GetAssociates(canDoLoadingQuery, loaders, loaderCountRequested);
LoadAssociatesInto(listBoxLoaders, associates);
sortedMaster.RemoveAll(associates);

// similiar code for canDoMHEQuery and canDoDivertsQuery

// finally
MessageBox.Show(@"You have " + sortedMaster.Count() + @" associates unassigned.");

The 3 lines for doing each of these could also be moved into a method as well so further refactoring would reduce the core code down to 4 lines.

Answer 3

As a first pass here is what I got. It is nowhere close to what it should be, but it is a start. If you gave more code, I could perfect it further but right now I do not know where the variables are coming from.

public void Sort()
{
    //Begin the sorting method
    if (loaders != loaderCountRequested) 
    {
        foreach (Associate someAssocVar in canDoLoadingQuery)
        {
            loadingList.Add(someAssocVar);
            loaders++;
            listBoxLoaders.Items.Add(loadingList[loaders - 1].FirstName + 
                " " + loadingList[loaders - 1].LastName);

        }
    }

    sortedMaster.RemoveAll(loadingList.Contains);
    if (mhes != mheCountRequested)
    {
        foreach (Associate someAssocVar in canDoMHEQuery)
        {    
            mheList.Add(someAssocVar);
            mhes++;
            listBoxMHE.Items.Add(mheList[mhes - 1].FirstName + 
                " " + mheList[mhes - 1].LastName);
        }
    }

    sortedMaster.RemoveAll(mheList.Contains);
    if (diverters != divertCountRequested)
    {
        foreach (Associate someAssocVar in canDoDivertsQuery)
        {
            divertsList.Add(someAssocVar);
            diverters++;
            listBoxDiverts.Items.Add(divertsList[diverters - 1].FirstName + 
                " " + divertsList[diverters - 1].LastName);
        }
    }

    sortedMaster.RemoveAll(divertsList.Contains);
    MessageBox.Show(@"You have " + sortedMaster.Count() + @" associates unassigned.");
}

Answer 4

I didnt check carefully but really 3 (at least) were because you didnt realize, know or remember break isn't only used in switch statements. The other one is really something suggesting if it should loop again or not.

You should run your code and my code in a diff tool to easily see the few lines that changed.

public void BtnRandomizeClick(object sender, EventArgs e)
{
    var DoAgain=true;
    while(DoAgain){
        DoAgain=false;
        switch (MessageBox.Show(@"Did you SAVE your edited list?", @"Just checking... It's important.",
                                MessageBoxButtons.YesNo))
        {
                #region Dialog Yes

            case DialogResult.Yes:
                if (FileChosen == null)
                {
                    MessageBox.Show(
                        @"Please select a file to load before trying to randomize. You need some entries to randomize first eh?",
                        "Oh fiddlesticks...");
                    return;
                }
                dataGridAssociates.Rows.Clear();
                var masterList = new List<Associate>();
                var divertsList = new List<Associate>();
                var mheList = new List<Associate>();
                var loadingList = new List<Associate>();
                var lines = File.ReadAllLines(FileChosen); //Array of all the lines in the text file

                foreach (var assocStringer in lines)
                {
                    if (assocStringer == null) continue;
                    var entries = assocStringer.Split('|');
                    var obj = (Associate) _bindingSource.AddNew();
                    if (obj == null) continue;
                    if (entries.Length < 7) break;
                    obj.FirstName = entries[0];
                    obj.LastName = entries[1];
                    obj.AssocId = entries[2];
                    obj.AssocRfid = entries[3];
                    obj.CanDoDiverts = Boolean.Parse(entries[4]);
                    obj.CanDoMhe = Boolean.Parse(entries[5]);
                    obj.CanDoLoading = Boolean.Parse(entries[6]);
                    obj.UniqueRandomID = Guid.NewGuid().ToString();
                    masterList.Add(obj);
                }
                var sortedMaster = masterList.OrderBy(d => d.UniqueRandomID).ToList();
                var loaderCountRequested = 0;
                var mheCountRequested = 0;
                var divertCountRequested = 0;
                var validDivertEntry = int.TryParse(txtDivertWorkers.Text, out divertCountRequested);
                var validMHEEntry = int.TryParse(txtMHEWorkers.Text, out mheCountRequested);
                var validLoaderEntry = int.TryParse(txtLoadersNeeded.Text, out loaderCountRequested);

                //See if whomever used the inputs correctly
                if (validDivertEntry == false)
                {
                    MessageBox.Show(@"Please use an integer in the 'Divert Workers' textbox and try again");
                    return;
                }
                if (validMHEEntry == false)
                {
                    MessageBox.Show(@"Please use an integer in the 'MHE Workers' textbox and try again");
                    return;
                }
                if (validLoaderEntry == false)
                {
                    MessageBox.Show(@"Please use an integer in the 'Loaders' textbox and try again");
                    return;
                }

                var canDoDivertsQuery = sortedMaster.Where(a => a.CanDoDiverts);
                var canDoMHEQuery = sortedMaster.Where(b => b.CanDoMhe);
                var canDoLoadingQuery = sortedMaster.Where(c => c.CanDoLoading);
                var loaders = 0;
                var mhes = 0;
                var diverters = 0;

                //Begin the sorting method
                foreach (Associate someAssocVar in canDoLoadingQuery)
                {
                    if (loaders == loaderCountRequested)
                        break;
                    loadingList.Add(someAssocVar);
                    loaders++;
                    listBoxLoaders.Items.Add(loadingList[loaders - 1].FirstName + " " +
                                             loadingList[loaders - 1].LastName);

                }
                sortedMaster.RemoveAll(loadingList.Contains);

            foreach (Associate someAssocVar in canDoMHEQuery)
                {
                    if (mhes == mheCountRequested)
                        break;
                    mheList.Add(someAssocVar);
                    mhes++;
                    listBoxMHE.Items.Add(mheList[mhes - 1].FirstName + " " +
                                         mheList[mhes - 1].LastName);
                }

                sortedMaster.RemoveAll(mheList.Contains);

                foreach (Associate someAssocVar in canDoDivertsQuery)
                {
                    if (diverters == divertCountRequested)
                        break;
                    divertsList.Add(someAssocVar);
                    diverters++;
                    listBoxDiverts.Items.Add(divertsList[diverters - 1].FirstName + " " +
                                             divertsList[diverters - 1].LastName);
                }

                sortedMaster.RemoveAll(divertsList.Contains);

                MessageBox.Show(@"You have " + sortedMaster.Count() + @" associates unassigned.");
                break;
                #endregion

                #region Dialog No

            case DialogResult.No:
                if (MessageBox.Show(@"Would you like to save your work then?", @"It really does matter...",
                                    MessageBoxButtons.YesNo) == DialogResult.Yes)
                {
                    var saveDialog = new SaveFileDialog
                                         {
                                             InitialDirectory =
                                                 Convert.ToString(Environment.SpecialFolder.MyDocuments),
                                             // ReSharper disable LocalizableElement
                                             Filter = "Text (*.txt)|*.txt|All Files (*.*)|*.*",
                                             // ReSharper restore LocalizableElement
                                             FilterIndex = 1
                                         };
                    if (saveDialog.ShowDialog() == DialogResult.OK)
                    {
                        FileChosen = saveDialog.FileName;
                        var assocToText = DatagridToString(dataGridAssociates, '|');
                        using (var outfile = new StreamWriter(saveDialog.FileName))
                            outfile.Write(assocToText);
                    }
                    DoAgain=true;
                }
                break;

                #endregion
        }
    }
}

Answer 5

Another tip. You can write code like this which will make it easier to move chucks into functions.

{
    int heyGuessWhatICanDeclThisAgainLater=1;
    blah blah blah
    if (blah != blahblahblah)
    {
        blah
    }
}
{
    bool heyGuessWhatICanDeclThisAgainLater= true;//not even the same type
    //but confusing if a dif type. But good for single letter loop vars/temps
    //like foreach(var v in list) { ...
    if (blah == blahblahblah || blu blu blah)
    {
        blah
    }
}

Also you shouldnt create so many variables (or reuse any vars. assign once is a good idea). You should just write

foreach (Associate someAssocVar in sortedMaster.Where(a => a.CanDoDiverts))

Its confusing to see code declare variables one place and used WAYYYY LATER.