0

Have in mind that this problem probably is the consequence of a beginners-mistake.

There are there 4 classes in my program relevant for this problem.

Main form: Declares and initiates the object currentRecipe (uses class recipe) and recipeMngr (uses class recipeManager) directly in the class. currentRecipe(Recipe) have a couple of fields that is collected from user input, the fields include the array ingredients[] that is collected from a property in another form that opens when a button is pressed. currentRecipe is used later when "add recipe button" is pressed and because it is used in both these methods the object currentRecipe needs to be initialized outside these methods as i understand it.

RecipeManager: Hold an array that stores the recipes. And method that that manages the adding of the recipe into the array. this method takes currentRecipe as a property from the main form.

Recipe: Holds the template for Recipe.

FormIngredients: collect ingredients from user and stores them in property.

However, the problem. When storing an recipe in the array in recipeManager the latest stored array just copies so all earlier assigned items in the list gets the new value. As an result

recipe[0] = Waffles

recipe[1] =

...

when adding a new it becomes

recipe[0] = pie

recipe[1] = pie ...

When it should become

recipe[0] = Waffles

recipe[1] = pie

...

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        InitializeGUI();

    }

    private const int maxRecipe = 20;
    private const int maxIngredients = 50;


    private static Recipe currentRecipe = new Recipe(maxIngredients);
    private static RecipeManager recipeMngr = new RecipeManager(maxRecipe);

private void btnAddIngredients_Click(object sender, EventArgs e)
    {

        FormIngredients FormI = new FormIngredients(currentRecipe);

        var result = FormI.ShowDialog();//show ingredient form

        if (result == DialogResult.OK) {
            currentRecipe = FormI.recipe;
            lblIngredAmount.Text = "Ingredients: " + currentRecipe.ingredientsAmounts().ToString();

        }
    }


private void AddRecipe(Recipe scurrentRecipe) {
        scurrentRecipe.rName = tbxName.Text;
        scurrentRecipe.rDescription = tbxDescription.Text;
        scurrentRecipe.rCategory = (FoodCategories)cbxCategory.SelectedIndex;


        bool valid = checkIfValid();
        if (valid)
        {
            recipeMngr.Add(scurrentRecipe);
            updateGUI();
            SimpleInitializeGUI();

        }
        else
        {
            MessageBox.Show("Please fill all the fields (or add atleast one ingredient)", "Stop");
        }
    }

I've added the most important parts from main form, where is know the problem is from.

I want to add that the problem disappears when moving the initialization of currentRecipe into the "addRecipe click"-method. But this creates alot of new problems and the code is supposed to be structured like this.

I do not use any loop to fill the array.

Martin
  • 1
  • 1
  • 4
    `scurrentRecipe` is not the current recipe object - it is the *only* recipe and you are changing its contents and adding it as if it was new – Ňɏssa Pøngjǣrdenlarp Jan 30 '17 at 19:02
  • Used up my duplehammer on wrong duplicate. Correct duplicate - http://stackoverflow.com/questions/2156482/why-does-adding-a-new-value-to-list-overwrite-previous-values-in-the-list – Alexei Levenkov Jan 30 '17 at 19:03
  • Besides what Plutonix said, what's the Add method of RecipeManager? @AlexeiLevenkov voted for you – Camilo Terevinto Jan 30 '17 at 19:03
  • Possible duplicate of [Why does adding a new value to list<> overwrite previous values in the list<>](http://stackoverflow.com/questions/2156482/why-does-adding-a-new-value-to-list-overwrite-previous-values-in-the-list) – Camilo Terevinto Jan 30 '17 at 19:04
  • Adding the scurrentRecipe was just one of my desperate attempts to make this work, removing it and just using currentRecipe doesnt work either. – Martin Jan 30 '17 at 19:07
  • I think i have come to a conclusion that that the add method in recipeManager is not relevant, i've changed in different ways including using array list instead of regular array. Nothing different – Martin Jan 30 '17 at 19:15
  • "the problem disappears when moving the initialization of currentRecipe into the "addRecipe click"-method. But this creates alot of new problems" - _Those_ problems need to be solved, or you need to make a _copy_ of the object before adding it to the collection. You are currently adding the same instance to the collection (whether it's a list or array is not relevant). – D Stanley Jan 30 '17 at 20:40
  • Core issue is that `AddRecipe()` should contain a `new Recipe()` somehow. – H H Jan 30 '17 at 20:46
  • Why Henk Holterman? – Martin Jan 30 '17 at 21:07
  • I have tried copying and it didn't work, i'm sorry but i don't understand "adding the same instance". I'm adding the updated instance right? Everytime the button is pressed new information goes into currentRecipe right? – Martin Jan 30 '17 at 21:14
  • I've done some more debugging and it appears as if the array turns completely empty just after the instance is added into recipeMngr. It doesn't do this when the currentRecipe is created in the "click" -method. Why? this is driving me crazy. Thanks for the answers – Martin Jan 30 '17 at 21:17
  • I don't see where AddRecipe is called from the BtnClick... But learn to think about _instances_ . Every time your user adds a recipe, your code needs to execute a `new Recipe(...)`. Also, get rid of the `static` elements. You don't need them and they are a bad practice. – H H Jan 31 '17 at 07:38
  • Learning about instances and objects solved the issue, thanks alot! Making a "deep copy" was the way to go, instead of doing recipe1 = recipe2 which will just make them reference the same spot in the storage. – Martin Jan 31 '17 at 22:25

1 Answers1

0

Explanation: Classes are reference types. If you go through the original code you've posted, the currentRecipe is instantiated only once. Everytime you are changing the value of scurrentRecipe / currentRecipe variables, you are just changing that object only. If you need multiple objects, you've to create them by multiple times by using the new keyword.

Updated code to make it clear for you:

public partial class Form1 : Form
{
public Form1()
{
    InitializeComponent();
    InitializeGUI();

}

private const int maxRecipe = 20;
private const int maxIngredients = 50;


private static Recipe currentRecipe = new Recipe(maxIngredients);
private static RecipeManager recipeMngr = new RecipeManager(maxRecipe);

private void btnAddIngredients_Click(object sender, EventArgs e)
{

    FormIngredients FormI = new FormIngredients(currentRecipe);

    var result = FormI.ShowDialog();//show ingredient form

    if (result == DialogResult.OK) {
        currentRecipe = FormI.recipe;
        lblIngredAmount.Text = "Ingredients: " + currentRecipe.ingredientsAmounts().ToString();

    }
}


private void AddRecipe() {
    scurrentRecipe = new Recipe(maxIngredients);
    scurrentRecipe.rName = tbxName.Text;
    scurrentRecipe.rDescription = tbxDescription.Text;
    scurrentRecipe.rCategory = (FoodCategories)cbxCategory.SelectedIndex;


    bool valid = checkIfValid();
    if (valid)
    {
        recipeMngr.Add(scurrentRecipe);
        updateGUI();
        SimpleInitializeGUI();
        currentRecipe = scurrentRecipe();
    }
    else
    {
        MessageBox.Show("Please fill all the fields (or add atleast one ingredient)", "Stop");
    }
}
CodeNinja
  • 94
  • 1
  • 6
  • Tried this before but didn't realize how c# worked with instances. learned a bit about that and "deep copying" and managed to solve the issue after a while. Thank you i'm grateful! – Martin Jan 31 '17 at 22:22