0

I have a small winforms program for a task which is to create a Recipe book. The application has two windows, but my bug pertains to only one.

I have two main classes: Recipe.cs and RecipeManager.cs, the idea is that the RecipeManager holds an array of Recipes. (For this task we weren't allowed to use Linq or ArrayLists)

Now, when I fill in the form and click "Add Recipe", the first one works successfully and the listBox populates correctly, however the second time I "Add Recipe" the recipeList array seems to get rewritten with the new Recipe entirely, and I have no idea why this is happening. The recipeList seems to change before my Add method even adds it to the array!

See gif of problem in question:

https://i.imgur.com/sIMICcG.gifv

Recipe.cs

using System;
using System.Collections.Generic;
using System.Text;
using System.Threading.Tasks;

namespace programmingTask4
{
    public class Recipe
    {
        private string[] ingredientArray;
        private string name;
        private FoodCategory category;
        private string description;
        private const int maxNumOfIngredients = 20;

       public string Name
        {
            get { return name; }
            set { name = value; }
        }
        public FoodCategory Category
        {
            get { return category; }
            set { category = value; }
        }
        public string Description
        {
            get { return description; }
            set { description = value; }
        }
        public string[] Ingredient
        {
            get { return ingredientArray; }
            set { ingredientArray = value; }
        }
        public int MaxNumOfIngredients
        {
            get { return ingredientArray.Length; }
        }

        public Recipe(int maxNumOfIngredients)
        {
            Console.WriteLine("Recipe constructor was called!");
            ingredientArray = new string[maxNumOfIngredients];
            DefaultValues();
        }
        public void DefaultValues()
        {
            for (int i = 0; i < ingredientArray.Length; i++)
            {
                ingredientArray[i] = string.Empty;
                name = string.Empty;
                category = FoodCategory.Vegetarian;
                description = string.Empty;
            }
        }
        public int FindVacantPosition()
        {
            int results;
            for (int i = 0; i < ingredientArray.Length; i++)
            {
                if(ingredientArray[i] == string.Empty)
                {
                    results = i;
                    return results;
                }
            }
            return -1;
        }
        public bool AddIngredient(string value)
        {
            bool ok;
            int next = FindVacantPosition();
            if(next >= 0)
            {
                ok = true;
                ingredientArray[next] = value;
            }
            else { 
            ok = false ;
            }
            return ok;
        }
        public bool CheckIndex(int index)
        {
            bool check = false;
            if(index <= ingredientArray.Length && index >= 0)
            {
                check = true;
            }
            return check;
        }
        public int GetCurrentNumOfIngredients()
        {
            int count = 0;
            for (int i = 0; i < ingredientArray.Length; i++)
            {
                if (!string.IsNullOrEmpty(ingredientArray[i]))
                {
                    count++;
                }
            }
            return count;
        }
        public override string ToString()
        {
            int chars = Math.Min(description.Length, 15);
            string descriptionText = description.Substring(0, chars);

            if (string.IsNullOrEmpty(descriptionText))
                descriptionText = "NO DESCRIPTION";

            string textOut = string.Format("{0, -20} {1,4} {2,-12} {3,-15}", name, GetCurrentNumOfIngredients(), category.ToString(), descriptionText);
            return textOut;
        }
        public bool ChangeIngredientAt(int index, string value)
        {
            bool bok = true;
            if (CheckIndex(index))
                ingredientArray[index] = value;
            else
                bok = false;
            return bok;
        }
        public bool DeleteIngredientAt(int index)
        {
            bool bok = true;
            if (CheckIndex(index))
                ingredientArray[index] = "NO DESCRIPTION";
            else
                bok = false;
            return bok;
        }


    }
}

RecipeManager.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace programmingTask1
{
    class RecipeManager
    {
        private Recipe[] recipeList;

        public RecipeManager(int maxNumOfElements)
        {
            Console.WriteLine("Recipe manager constructor called!");
            recipeList = new Recipe[maxNumOfElements];
        }

        private int FindVacantPosition()
        {
            for (int i = 0; i < recipeList.Length; i++)
            {
                if (recipeList[i] == null)
                {
                    Console.WriteLine("Found free position at: " + i);
                    return i;
                }

            }
            return -1;
        }
        public bool CheckIndex(int index)
        {
            bool check = false;
            if (index <= recipeList.Length && index >= 0)
            {
                check = true;
            }
            return check;
        }
        public Recipe GetRecipeAt(int index)
        {
            if (CheckIndex(index))
                return recipeList[index];
            else
                return null;
        }
        public bool Add(Recipe newRecipe)
        {
            if (newRecipe == null)
                return false;

            bool ok;
            int next = FindVacantPosition();
            if (next >= 0)
            {
                ok = true;
                Console.WriteLine("Setting recipe list at index " + next + " to " + newRecipe.ToString());
                recipeList[next] = newRecipe;

            }
            else
            {
                Console.WriteLine("No space for recipe available! " + next);
                ok = false;
            }
            return ok;
        }

        public int CurrentNumberofItems()
        {
            int num = 0;
            for (int i = 0; i < recipeList.Length; i++)
            {
                if (recipeList[i] != null)
                {
                    num++;
                }
            }
            return num;
        }
        public string[] RecipeListToString()
        {
            string[] results = new string[recipeList.Length];
            for (int i = 0; i < recipeList.Length; i++)
            {
                if (recipeList[i] != null)
                    results[i] = recipeList[i].ToString();
                else
                    results[i] = string.Empty;
            }
            return results;
        }
    }
}

FormMain.cs

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace programmingTask4
{
    public partial class FormMain : Form
    {
        const int maxRecipe = 3;
        const int maxIngredients = 20;
        Recipe currRecipe = new Recipe(maxIngredients);
        RecipeManager recipemanager = new RecipeManager(maxRecipe);

        public FormMain()
        {
            Console.WriteLine("Form constructor was called!");
            InitializeComponent();
            InitializeGui();
        }

        public void InitializeGui()
        {
            comboBoxCategory.DataSource = Enum.GetValues(typeof(FoodCategory));
        }
        public void UpdateGUI()
        {
            //listBoxDisplay.Text = recipemanager.CurrentNumberofItems().ToString();
            //listBoxDisplay.Text = currRecipe.Ingredient.ToString();

            string[] recipeListStrings = recipemanager.RecipeListToString();
            listBoxDisplay.Items.Clear();
            listBoxDisplay.Items.AddRange(recipeListStrings);
        }

        private void buttonRecipe_Click(object sender, EventArgs e)//add recipe button
        {
            currRecipe.Category = (FoodCategory)comboBoxCategory.SelectedIndex;
            currRecipe.Name = textBoxRecipeName.Text.Trim();
            currRecipe.Description = richTextBoxDescription.Text.Trim();

            Console.WriteLine("Adding recipe: " + currRecipe.ToString());
            bool result = recipemanager.Add(currRecipe);
            Console.WriteLine("Result was " + result + " for adding to recipe list");
            UpdateGUI();

            currRecipe.DefaultValues();   
        }
    }
}
Cherona
  • 758
  • 2
  • 10
  • 27
  • **[Using the free, built-in Step Debugger](https://msdn.microsoft.com/en-us/library/y740d9d3.aspx)** to debug your code is easier than you think. It will also help you learn how code executes which will help you write better code. – Ňɏssa Pøngjǣrdenlarp Mar 18 '20 at 16:10
  • I added breakpoints all over my code, and slowly stepped through (and into). The recipeList changes as soon as Add gets called, but before Add gets a chance to do anything. @ŇɏssaPøngjǣrdenlarp – Cherona Mar 18 '20 at 16:13
  • For one thing, you are making it more complicated than it needs to be. Databinding would simplify the display aspects and modern collections already provide all the functionality you are replicating to Find and Check things. Also you are not creating a new recipe item in that click event where Add is called. Then lines which update props are changing the last item and adding it again – Ňɏssa Pøngjǣrdenlarp Mar 18 '20 at 16:19
  • @Cherona study about [reference type](https://stackoverflow.com/questions/18229463/reference-type-in-c-sharp) and you will get it. – Louis Go Mar 18 '20 at 16:22

1 Answers1

1
  1. Recipe is a class, which mean it's a reference type.
  2. In your main form, your currRecipe instance is never changed.
  3. RecipeManager has an array to store references of instance but unfortunately it stores the same instance because of 2.
  4. Since RecipeManager stores the same instance of currRecipe, any modification on currRecipe would display N times.

To prevent it. Modify your buttonRecipe_Click

    private void buttonRecipe_Click(object sender, EventArgs e)//add recipe button
    {
        currRecipt = new Recipe(maxIngredients);
        currRecipe.Category = (FoodCategory)comboBoxCategory.SelectedIndex;
        currRecipe.Name = textBoxRecipeName.Text.Trim();
        currRecipe.Description = richTextBoxDescription.Text.Trim();

        Console.WriteLine("Adding recipe: " + currRecipe.ToString());
        bool result = recipemanager.Add(currRecipe);
        Console.WriteLine("Result was " + result + " for adding to recipe list");
        UpdateGUI();

        // No need to reset it, use new one everytime.
        //currRecipe.DefaultValues();   
    }
Louis Go
  • 2,213
  • 2
  • 16
  • 29