Getting the max value from a list of sums This is my version of the knapsack challenge and I was wondering if there was a way to do it so it doesn't go through the foreach statement so many times. At the minute it goes through it thousands of times to get he closes value to the target number, which takes long time to do, I was wondering if there was a way to either do it without foreach or make it quick with the foreach.

private void btnGenerate_Click(object sender, RoutedEventArgs e)
{
    string input;
    string bag;
    int goal;
    int count = 0;

    int element;
    List&lt;int&gt; finalResult = new List&lt;int&gt;();
    do
    {
        bag = txtBag.Text;
    }
    while (!int.TryParse(bag, out goal));
    input = txtGenerate.Text;
    string[] elementsText = input.Split(' ');
    List&lt;int&gt; elementsList = new List&lt;int&gt;();
    foreach (string elementText in elementsText)
    {

        if (int.TryParse(elementText, out element))
        {
            elementsList.Add(element);
        }
    }
    Solver solver = new Solver();
    List&lt;List&lt;int&gt;&gt; results = solver.Solve(goal, elementsList.ToArray());
    int closestValue = 0;
    foreach (List&lt;int&gt; result in results)
    {
        count++;
        int totalValue = result.Sum(x =&gt; Convert.ToInt32(x));

        if (totalValue &gt;= closestValue)
        {
            closestValue = totalValue;
            finalResult = result;
            txtSum.Text = closestValue.ToString();

        }
    }

    foreach (int value in finalResult)
    {
        txtCount.Text = count.ToString();

        cboResult.ItemsSource = finalResult;

    }
}


I also have a class with this in:

public class Solver
{
    public List&lt;List&lt;int&gt;&gt; mResults;
    public int closestValue = 0;
    public List&lt;List&lt;int&gt;&gt; Solve(int goal, int[] elements)
    {
        mResults = new List&lt;List&lt;int&gt;&gt;();
        RecursiveSolve(goal, 0, new List&lt;int&gt;(), new List&lt;int&gt;(elements), 0);
        return mResults;
    }

    private void RecursiveSolve(int goal, int currentSum,
    List&lt;int&gt; included, List&lt;int&gt; notIncluded, int startIndex)
    {
        for (int index = startIndex; index &lt; notIncluded.Count; index++)
        {
            int nextValue = notIncluded[index];
            if (currentSum + nextValue &lt;= goal &amp;&amp; currentSum + nextValue &gt;= closestValue)
            {
                List&lt;int&gt; newResult = new List&lt;int&gt;(included);
                newResult.Add(nextValue);
                mResults.Add(newResult);
                closestValue = currentSum + nextValue;
            }
            if (currentSum + nextValue &lt; goal)
            {

                List&lt;int&gt; nextIncluded = new List&lt;int&gt;(included);
                nextIncluded.Add(nextValue);
                List&lt;int&gt; nextNotIncluded = new List&lt;int&gt;(notIncluded);
                nextNotIncluded.Remove(nextValue);
                RecursiveSolve(goal, currentSum + nextValue,
                    nextIncluded, nextNotIncluded, startIndex++);
            }
        }
    }
}
You've got logic mixed in with your view.

Move as much of the code out of your button handler as you can, and into a separate function (and preferably a separate class).

A button handler should do just that, handle button presses. It should not then perform calculations.



Your spacing could use some work. For example:

    int element;
    List&lt;int&gt; finalResult = new List&lt;int&gt;();
    do
    {
        bag = txtBag.Text;
    }
    while (!int.TryParse(bag, out goal));
    input = txtGenerate.Text;
    string[] elementsText = input.Split(' ');
    List&lt;int&gt; elementsList = new List&lt;int&gt;();
    foreach (string elementText in elementsText)
    {

        if (int.TryParse(elementText, out element))
        {
            elementsList.Add(element);
        }
    }
    Solver solver = new Solver();
    List&lt;List&lt;int&gt;&gt; results = solver.Solve(goal, elementsList.ToArray());
    int closestValue = 0;
    foreach (List&lt;int&gt; result in results)
    {
        count++;
        int totalValue = result.Sum(x =&gt; Convert.ToInt32(x));

        if (totalValue &gt;= closestValue)
        {
            closestValue = totalValue;
            finalResult = result;
            txtSum.Text = closestValue.ToString();

        }
    }


Would be a lot more readable like this:

    int element;
    List&lt;int&gt; finalResult = new List&lt;int&gt;();

    do
    {
        bag = txtBag.Text;
    }
    while (!int.TryParse(bag, out goal));

    input = txtGenerate.Text;
    string[] elementsText = input.Split(' ');
    List&lt;int&gt; elementsList = new List&lt;int&gt;();

    foreach (string elementText in elementsText)
    {

        if (int.TryParse(elementText, out element))
        {
            elementsList.Add(element);
        }
    }

    Solver solver = new Solver();
    List&lt;List&lt;int&gt;&gt; results = solver.Solve(goal, elementsList.ToArray());
    int closestValue = 0;

    foreach (List&lt;int&gt; result in results)
    {
        count++;
        int totalValue = result.Sum(x =&gt; Convert.ToInt32(x));

        if (totalValue &gt;= closestValue)
        {
            closestValue = totalValue;
            finalResult = result;
            txtSum.Text = closestValue.ToString();

        }
    }




Use var when it is obvious from the RHS of a declaration what the type of the variable is, and always use var in foreach loops.

e.g.

    int element;
    var finalResult = new List&lt;int&gt;();

    do
    {
        bag = txtBag.Text;
    }
    while (!int.TryParse(bag, out goal));

    input = txtGenerate.Text;
    var elementsText = input.Split(' ');
    var elementsList = new List&lt;int&gt;();

    foreach (var elementText in elementsText)
    {

        if (int.TryParse(elementText, out element))
        {
            elementsList.Add(element);
        }
    }

    var solver = new Solver();
    List&lt;List&lt;int&gt;&gt; results = solver.Solve(goal, elementsList.ToArray());
    var closestValue = 0;

    foreach (var result in results)
    {
        count++;
        int totalValue = result.Sum(x =&gt; Convert.ToInt32(x));

        if (totalValue &gt;= closestValue)
        {
            closestValue = totalValue;
            finalResult = result;
            txtSum.Text = closestValue.ToString();
        }
    }




Avoid Hungarian notation in C#, and prefer public properties over public fields. Additionally, public collections should be readonly, and you don't need to intiialize an integer as 0. Its default value is 0.

    private readonly List&lt;List&lt;int&gt;&gt; results

    public List&lt;List&lt;int&gt;&gt; Results 
    {
        get 
        {
            return results;
        }
    }

    public int ClosestValue {get; set;}