2

I have a struct holding condition informations.

private struct hintStructure
{
    public string id;
    public float  value;
    public bool   alreadyWarned;
}
private List<hintStructure> hints;

Whenever my program changes a value, an event is sent and the condition list is checked wether that element meets a condition.

public void EventListener (string id)
{
    CheckHints(id); //id of the updated element
}

private void CheckHints(string _id) 
{
    foreach (hintStructure _h in hints)
        if (_h.id == _id) CheckValue(_h);
}

private void CheckValue(hintStructure _h)
{
    float _f = GetValue(_h.id);

    if (_f < _h.value)
    {
        ActivateHint(_h);
        _h.alreadyWarned = true;
    }
    else 
        _h.alreadyWarned = false;
}

private void ActivateHint(hintStructure _h)
{
    if(_h.alreadyWarned == false)
        ShowPopup();
}

ShowPopup() should only be called, when it wasn't already shown for that specific element (indicated by bool alreadyWarned). Problem is: It's always shown. It seems like the line _h.alreadyWarned = true; is called, but the value isn't stored (I checked if it gets called, it does). I assumed the foreach might be the problem (since it was buggy some years ago), but it doesn't work with a for() structure either.

My last guess is an addressing issue, typical problem in C++: CheckValue(h); vs. CheckValue(&h); But if I'm correct with this guess - how can I solve this?

Jonas Zimmer
  • 161
  • 2
  • 15
  • 6
    The problem isn't `foreach` itself - it's your use of mutable structs. *Don't do that*. Beyond that, it's very hard to see what's wrong when you haven't shown the loop that's supposedly causing the problem. Please include a [mcve], and then stop using mutable structs... – Jon Skeet Mar 04 '16 at 14:44
  • 3
    C++ != C#. In other words, you should generally not be using `struct` in C# until you really understand the difference and need any performance improvements that may be gained. – crashmstr Mar 04 '16 at 14:44
  • @JonSkeet its not that he is addressing a struct as mutable - its because hes changing the struct on the top of the stack expecting that change to persist beyond the stack call. – PhillipH Mar 04 '16 at 14:46
  • 1
    @PhillipH: Well, we suspect that's the case. But we can't see the code, which makes it harder to tell for sure. (That's certainly a problem in `CheckValue`, admittedly.) But what I *am* pretty sure about is that if you avoid mutable structs (and public fields!) it's harder to get into this sort of situation. – Jon Skeet Mar 04 '16 at 14:46
  • 1
    I'd also strongly advise starting to follow .NET naming conventions, too... – Jon Skeet Mar 04 '16 at 14:47
  • 1
    It boils down to : mutable structs are **evil** – Falanwe Mar 04 '16 at 14:49
  • Thanks! C# indeed is not C++, the use of structs results from my C++ based education. I won't do it anymore! Changing the struct to a class solved the problem. @Jon Skeet: I'll have a look in the .NET naming conventions. ;) – Jonas Zimmer Mar 04 '16 at 14:54

2 Answers2

5

You are passing a struct, not a class instance. Changes to a struct (not passed as ref) are changing the local copy of the struct. You are trying to make the struct as a reference type.

Changes made to a struct within a function do not change the value of the originating struct - the struct is copied to the stack before being passed up to the function.

PhillipH
  • 6,182
  • 1
  • 15
  • 25
5

The problem is structs in C# are value type, so when you call CheckValue(_h) you are creating a copy of _h and that copy is updated, but original in the list remains intact.

The classic solution is passing this instance by reference (ref), but you can't do it with foreach variables.

The solution is change hintStructure to class:

private class Hint
{
    public string id;
    public float  value;
    public bool   alreadyWarned;
}
Arturo Menchaca
  • 15,783
  • 1
  • 29
  • 53