StolenSheep
StolenSheep

Reputation: 57

C# Static counter not incrementing - why?

New to passing static counters and I'm stuck. I want to count the instances of the Member() class and then put that value as the Member's Id number when I add them to an ObservableCollection, called _members in this case.

In my Member class:

    // Total numbers of instances that have been created
    public static int NumMembers { get; protected set; }

    // Static constructor initializes NumMembers
    static Member()
    {
        NumMembers = 0;
    }
    public Member(string img, int mbrnum, string mbrname, string phonenum, string mbradd, List<string> rtls)
    {
        // Increment member count and assign member number.
        NumMembers++;
        MemberNumber = NumMembers;

        Img = img;
        MemberName = mbrname;
        MemberPhone = phonenum;
        MemberAddress = mbradd;
        Rentals = rtls;
    } // Working ctor

In my main window:

        _members.Add(new Member()
        {
            Img = "Default.jpg",
            MemberNumber = 0,
            MemberName = "Joe Snow",
            MemberPhone = "0549880974",
            MemberAddress = "Mars",
            Rentals = new List<string>()
            {
                "Mrs. Brown's Boys D'Movie",
                "Jersey Boys"
            }
        });

If I hard-code the MemberNumber to any number in the main window it displays correctly so I know I'm doing something wrong here but I'm honestly a bit lost as to what's the snag. A fresh pair of eyeballs with more experience than me would be very much appreciated!

Upvotes: 1

Views: 3778

Answers (3)

Thorsten Dittmar
Thorsten Dittmar

Reputation: 56697

The fact you call a constructor first and then set the properties probably overwrites the correct member number with 0, no matter which constructor you call. Also, you're not calling the constructor that actually increments the NumMembers property, but the parameterless constructor.

The proper solution to your problem

Make the MemberNumber property read only and make sure it is only set upon construction of the object. That way you will prevent the property from accidentially being overwritten with other values.

Make this the parameterless constructor:

public Member()
{
    // Increment member count and assign member number.
    NumMembers++;
    MemberNumber = NumMembers;
}

It increments the static property and assigns the new member number to this instance. This is the only time the MemberNumber property is assigned and it actually also the only time the NumMembers property may be incremented!.

Make sure to call this constructor from all other constructors available:

public Member(string img, ...) : this()
{
    Img = img;
    ...
}

Note the : this() part!

Declare the MemberNumber property as follows to make sure it can not be changed from "outside":

public int MemberNumber
{
    get;
    private set;
}

Upvotes: 1

Van
Van

Reputation: 610

You incremented the counter in the constructor that takes parameters. But the way you are instantiating the Member object you are calling the default/empty constructor so you counter is never updated. You should call the constructor that you created or you should add the constructor in the default constructor and all subsequent constructors should call the default as well.

Below I added the counter int default constructor and made the subesquent constructor call it

// Total numbers of instances that have been created
    public static int NumMembers { get; protected set; }

    // Static constructor initializes NumMembers
    static Member()
    {
        NumMembers = 0;
    }

    public Member()
    {
        // Increment member count and assign member number.
        NumMembers++;
    }
    public Member(string img, int mbrnum, string mbrname, string phonenum, string mbradd, List<string> rtls)
:this()
    {
        MemberNumber = NumMembers;
        Img = img;
        MemberName = mbrname;
        MemberPhone = phonenum;
        MemberAddress = mbradd;
        Rentals = rtls;
    } // Working ctor

`

Upvotes: 1

Askolein
Askolein

Reputation: 3378

Call the correct constructor:

_members.Add(new Member("Default.jpg", 0, "Joe Snow", "0549880974", "Mars", 
    new List<string>
    {
        "Mrs. Brown's Boys D'Movie",
        "Jersey Boys"
    }));

But it seems that the second parameter mbrnum is useless since you are handling this counter in a static member, which is by the way highly not recommended for thread safety issues (but this is not your question here).

Upvotes: 0

Related Questions