Getting always a zero when I execute - c#

Question

So I'm doing something stupid in my class solution because I am expecting a number but I get a zero. The code is as follows:

class Spel
{
    private int _nummer1;
    public int Nummer1
    {
        get { return _nummer1; }
        set { _nummer1 = value; }
    }
    private int _nummer2;
    public int Nummer2
    {
        get { return _nummer2; }
        set { _nummer2 = value; }
    }
    private int _nummer3;
    public int Nummer3
    {
        get { return _nummer3; }
        set { _nummer3 = value; }
    }
    private int _punten;
    public int Punten
    {
        get { return _punten; }
        set { _punten = value; bepaal(); }
    }
    public void bepaal()
    {
        Random rnd = new Random();
        _nummer1 = rnd.Next(1, 4);
        _nummer2 = rnd.Next(1, 4);
        _nummer3 = rnd.Next(1, 4);
        if (_nummer1 == _nummer2 && _nummer2 == _nummer3 && _nummer1 == _nummer3)
        {
            _punten = 10;
        }
        else if (_nummer1 == _nummer2 && _nummer2 == _nummer3)
        {
            _punten = 5;
        }
        else if (_nummer1 == _nummer2 && _nummer1 == _nummer3)
        {
            _punten = 5;
        }
        else if (_nummer2 == _nummer3 && _nummer1 == _nummer3)
        {
            _punten = 5;
        }
    }

I think the fault is in the area where I give the 'nummer' a random number. Here is the code where I call the class with a controller:

public class Controller
{
    private Spel spl = new Spel();
    public int getNummer1()
    {
        return spl.Nummer1;
    }

In the next code I'm using the controller to bring up the class:

private void button1_Click(object sender, EventArgs e)
    {
        teller++;
        label1.Text = spl.getNummer1().ToString(); 
....

I don't know where the fault is so, can someone help me?

What value would you expect to see? You never assign a value to `Number`, so the setter never gets called. The setter never gets called, so `_number` remains at its default value: 0. Your setter also doesn't make any sense, because you set the backing field (`_number`) to the value assigned to the property, but then immediately call `getNumber()` that just sets it to 10. — ProgrammingLlama, Oct 22 '22 at 14:17
Well, of course you get zero with the program code in your question. Anything else doesn't make sense, really... — , Oct 22 '22 at 14:17
As said, you can't _get_ anything than zero because that's the default for an integer value never touched by any _set_ action — Steve, Oct 22 '22 at 14:18
@Tuur, you can edit your question and correct any typos (and add any clarifications asked for). No need to explain your typos in the comments ;-) — , Oct 22 '22 at 14:18
"_I expect the number 10_" But that's not what the code is doing. Do not expect anything based on unproven assumptions. Discard your assumptions. Read your code. Read your program code line by line, very carefully (do not gloss over anything), method/property invocation by invocation to learn what it does. — , Oct 22 '22 at 14:19
Please can you edit your question to explain why you believe that `Console.WriteLine(i1.Number);` should print any value other than 0, because I'm not seeing it. — ProgrammingLlama, Oct 22 '22 at 14:20
P.S. I recommend you follow this tutorial from Microsoft: [Tutorial: Learn to debug C# code using Visual Studio](https://learn.microsoft.com/en-us/visualstudio/get-started/csharp/tutorial-debugger?view=vs-2022) and perhaps also Eric Lippert's [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). — ProgrammingLlama, Oct 22 '22 at 14:21
Just saw your edit: methods don't call themselves. You don't explicitly call `bepaal();` from `getNummer1()` and you don't assign a value to the `Punten` property, so the setter there doesn't call `bepaal();` either. Finally, you don't ever assign a number to `_nummer1` (directly, or indirectly through the `Nummer1` property's setter) outside of this `bepaal` method. As such, `_nummer1` retains its default value: 0. — ProgrammingLlama, Oct 22 '22 at 14:49
**Do not initialize `Random` at each function call**. This is a common mistake. It is recommended use store a single `Random` variable in a readonly static field to be initialized **only once**. — John Alexiou, Oct 22 '22 at 15:09
@JohnAlexiou: But be aware that `Random` isn't thread-safe (or at least, it wasn't a while ago; it's possible it's changed over time). — Jon Skeet, Oct 22 '22 at 16:34
About thread safety: [The System.Random class and thread safety](https://learn.microsoft.com/en-us/dotnet/api/system.random?redirectedfrom=MSDN&view=net-6.0#the-systemrandom-class-and-thread-safety) — Luuk, Oct 22 '22 at 16:38

score 1 · Answer 1 · answered Oct 22 '22 at 14:23

You can let the compiler do all of the heavy lifting regarding getters and setters in c# so you can change your code to:

public class Class1
    {
        public int Number {get; set;} = 10;
    }

And get the expected behavior. Try this out and get back if you still are having issues. If you want to learn more about this syntax check here

score 0 · Accepted Answer · answered Oct 22 '22 at 15:22

There are several issues with this code.

One of them has to do with the Punten properties, which is calculated and assigned a value at the same time.

The code below needs revising.

private int _punten;
public int Punten
{
    get { return _punten; }
    set { _punten = value; bepaal(); }  // BUG: assign _punten twice
}
public void bepaal()
{
    Random rnd = new Random();
    _nummer1 = rnd.Next(1, 4);
    _nummer2 = rnd.Next(1, 4);
    _nummer3 = rnd.Next(1, 4);
    if (_nummer1 == _nummer2 && _nummer2 == _nummer3 && _nummer1 == _nummer3)
    {
        _punten = 10;
    }
    else if (_nummer1 == _nummer2 && _nummer2 == _nummer3)
    {
        _punten = 5;
    }
    else if (_nummer1 == _nummer2 && _nummer1 == _nummer3)
    {
        _punten = 5;
    }
    else if (_nummer2 == _nummer3 && _nummer1 == _nummer3)
    {
        _punten = 5;
    }
    // missing else branch, major bug.
}

I recommend a property with a public getter and private setter instead. Also the values of the properties are all assigned at Bepaal() so there is no need for a public setter to exist.

class Spel
{
    static readonly Random rnd = new Random();

    public int Nummer1 { get; private set; }
    public int Nummer2 { get; private set; }
    public int Nummer3 { get; private set; }
    public int Punten { get; private set; }

    public void Bepaal()
    {
        Nummer1 = rnd.Next(1, 4);
        Nummer2 = rnd.Next(1, 4);
        Nummer3 = rnd.Next(1, 4);
        if (Nummer1 == Nummer2 && Nummer2 == Nummer3 && Nummer1 == Nummer3)
        {
            Punten = 10;
        }
        else if (Nummer1 == Nummer2 && Nummer2 == Nummer3)
        {
            Punten = 5;
        }
        else if (Nummer1 == Nummer2 && Nummer1 == Nummer3)
        {
            Punten = 5;
        }
        else if (Nummer2 == Nummer3 && Nummer1 == Nummer3)
        {
            Punten = 5;
        }
        else
        {
            Punten = 0;    // missing default
        }
    }
}
class Program
{
    static void Main(string[] args)
    {
        Spel spel = new Spel();
        spel.Bepaal();
        Console.WriteLine(spel.Punten);
    }
}

Also note that the Random variables needs to be initialized only once, and it is a mistake to call new Random() every time a random number is needed. It is common practice to store a Random variable as a static readonly field and to be initialized only once, as seen above.

Getting always a zero when I execute - c#

2 Answers2