CODEWITHSUNDEEP
c#arraysproperties
Magnus
Magnus

Reputation: 7833

Returning Array in Property If Setting Value in Get Accessor

private readonly float[] _location;

public Player()
{
    _location = new float[3];
}

public float[] Location
{
    get
    {
        _location[0] = Worker.Elements["lbXCoordinate"].Value;
        _location[1]= Worker.Elements["lbYCoordinate"].Value;
        _location[2] = Worker.Elements["lbZCoordinate"].Value;

        return _location;
    }
}

Two questions:

  1. I often find myself setting the value of a Property in the get Accessor (as seen above), as opposed to using a set Accessor. This way I ensure that whenever someone asks for the value, they get an updated version. Is that generally OK to do?

  2. I realize that returning an Array in a Property is not good practice (link), but I am concerned that if I return a ReadOnlyCollection, then I have to make a new instance of both that and a float every time someone asks for an updated value (see below). It will be asked for several times per second in my program, so that seems to be unwise with regards to performance. Is it okay to just ignore this rule in my case, as the value is actually set every time a user asks for it (so we won't have an issue in the case someone happens to overwrite the array)?

Using ReadOnlyCollection:

private ReadOnlyCollection<float> _location;

public Player()
{
}

public ReadOnlyCollection<float> Location
{
    get
    {
        _location = new ReadOnlyCollection<float>(new float[] {Worker.Elements["lbXCoordinate"].Value, Worker.Elements["lbXCoordinate"].Value, Worker.Elements["lbXCoordinate"].Value});

        return _location;
    }
}

Upvotes: 1

Views: 145

Answers (1)

thumbmunkeys
thumbmunkeys

Reputation: 20764

You can return a IReadOnlyCollection<float> instead, because the array implements this interface.

This is valid code:

private  _location[] = new float[3];

public IReadOnlyCollection<float> Location
{
    get
    {
        return _location;
    }
}

Regarding the setting of the value in the getter, this seems ok to me, as the overhead is likely minimal.

You just need to ensure some synchronization in case of different threads accessing Worker.Elements.

To ensure consistent values returned, you can sync on Worker.Elements whenever you try to read or write this variable:

get
{
   lock(Worker.Elements)
   {
      _location[0] = Worker.Elements["lbXCoordinate"].Value;
      _location[1]= Worker.Elements["lbYCoordinate"].Value;
      _location[2] = Worker.Elements["lbZCoordinate"].Value;

      return _location;
   }       
}

Upvotes: 1

Related Questions