Access of private setter is not prohibited-CodePudding

Problem Statement

There is a custom vector class:

namespace StackoverflowQuestion1
{
    public class MyVector
    {
        public float x;
        public float y;
        public float z;

        public MyVector(float x, float y, float z)
        {
            this.x = x;
            this.y = y;
            this.z = z;
        }
    }
}

There is an interface for anything that's movable, which means positions may change:

namespace StackoverflowQuestion1
{
    public interface IMovable
    {
        public string Name { get; }
        public MyVector Position { get; }
    }
}

Furniture is movable, therefore it implements the corresponding interface:

namespace StackoverflowQuestion1
{
    public class Furniture : IMovable
    {
        public string Name { get; private set; }
        public MyVector Position { get; private set; }

        public Furniture(string name, float x, float y, float z)
        {
            this.Name = name;
            this.Position = new MyVector(x, y, z);
        }
    }
}

Accessing the private getter of the Name is not possible, as expected. Accessing the private setter of Position is also not working, as expected. However, accessing the fields of Position is possible, as they are public.

using StackoverflowQuestion1;

class Program
{
    static void Main(string[] args)
    {
        Furniture F = new Furniture("Chair", 1f, 2f, 3f);
        F.Name = "Office chair"; // doesn't work, as expected
        F.Position = new MyVector(5f, 6f, 7f); // doesn't work, as expected
        F.Position.x = 5f; // works, unfortunately
        F.Position.y = 6f; // works, unfortunately
        F.Position.z = 7f; // works, unfortunately
    }
}

Question

How to make it impossible to change the furniture's position, without making the coordinates of MyVector private and, thus, inaccesible? I want to have encapsulation, by only letting Furniture members access the position, but MyVector will become useless in other places if its values can't be changed.

CodePudding user response：

A couple of points to make here:

By design you chose to make the fields public which means they are readily accessible from other classes. They are not private which is what the title implies. To force them to be read only use the readonly keyword

public class MyVector
{
    public readonly float x;
    public readonly float y;
    public readonly float z;

    public MyVector(float x, float y, float z)
    {
        this.x = x;
        this.y = y;
        this.z = z;
    }
}

Typically you won't expose the fields but instead use properties with getters defined only.

public class MyVector
{
    private readonly float x;
    private readonly float y;
    private readonly float z;

    public MyVector(float x, float y, float z)
    {
        this.x = x;
        this.y = y;
        this.z = z;
    }

    public float X { get => x; }
    public float Y { get => y; }
    public float Z { get => z; }
}

Furthermore, you can simplify things using auto-properties

public class MyVector
{

    public MyVector(float x, float y, float z)
    {
        this.X = x;
        this.Y = y;
        this.Z = z;
    }

    public float X { get; }
    public float Y { get; }
    public float Z { get; }
}

Finially, it recommended for value semantics where (x,y,z) will always go together to use struct declarations.

public readonly struct MyVector
{

    public MyVector(float x, float y, float z)
    {
        this.X = x;
        this.Y = y;
        this.Z = z;
    }

    public float X { get; }
    public float Y { get; }
    public float Z { get; }
}

As a side note, if you try to modify the contents of a struct exposed by a property, the C# is going to complain.

Consider this code

public struct MyVector
{
    public float x;
    public float y;
    public float z;

    public MyVector(float x, float y, float z)
    {
        this.x = x;
        this.y = y;
        this.z = z;
    }
}

public class Movable
{
    public Movable(MyVector position)
    {
        Position = position;
    }

    public MyVector Position { get; }
}

scr1

So even with by design allowing the contents of MyVector to be mutable (change), the compiler is going to stop you. This is because with struct types you have local copies of the data everywhere and by writing Position.x = 10f you would have modified a local copy of Position that exists in the scope where this is called, and not modified the original data.

In the question MyVector is a class and so Position.x = 10f modifies the original data and as stated this is undesirable behavior, so follow the steps above to disallow this behavior.

To make MyVector work well with other classes I often add the following functionality to such deflations. I add support for .ToString() with formatting and I add support for .Equals() (and == for structures) in order to be to write code like this:

static void Main(string[] args)
{
    var pos = new MyVector(1f, 1/2f, 1/3f);
    var m = new Movable(pos);

    if (m.Position == pos)
    {
        Console.WriteLine($"{m.Position:f2}");
        // (1.00,0.50,0.33)
    }
}

Notice the formatting with 2 decimals and the equality check.

here is the complete code that allows this for your reference

MyVector.cs

public readonly struct MyVector : IEquatable<MyVector>, IFormattable
{
    public MyVector(float x, float y, float z)
    {
        this.X = x;
        this.Y = y;
        this.Z = z;
    }

    public float X { get; }
    public float Y { get;  }
    public float Z { get;  }


    #region IEquatable Members
    /// <summary>
    /// Equality overrides from <see cref="System.Object"/>
    /// </summary>
    /// <param name="obj">The object to compare this with</param>
    /// <returns>False if object is a different type, otherwise it calls <code>Equals(MyVector)</code></returns>
    public override bool Equals(object obj)
    {
        if (obj is MyVector other)
        {
            return Equals(other);
        }
        return false;
    }

    public static bool operator ==(MyVector target, MyVector other) { return target.Equals(other); }
    public static bool operator !=(MyVector target, MyVector other) { return !(target == other); }


    /// <summary>
    /// Checks for equality among <see cref="MyVector"/> classes
    /// </summary>
    /// <param name="other">The other <see cref="MyVector"/> to compare it to</param>
    /// <returns>True if equal</returns>
    public bool Equals(MyVector other)
    {
        return X.Equals(other.X)
            && Y.Equals(other.Y)
            && Z.Equals(other.Z);
    }

    /// <summary>
    /// Calculates the hash code for the <see cref="MyVector"/>
    /// </summary>
    /// <returns>The int hash value</returns>
    public override int GetHashCode()
    {
        unchecked
        {
            int hc = -1817952719;
            hc = (-1521134295) * hc   X.GetHashCode();
            hc = (-1521134295) * hc   Y.GetHashCode();
            hc = (-1521134295) * hc   Z.GetHashCode();
            return hc;
        }
    }

    #endregion

    #region Formatting
    public override string ToString() => ToString("g");
    public string ToString(string formatting) => ToString(formatting, null);
    public string ToString(string format, IFormatProvider provider)
    {
        return $"({X.ToString(format, provider)},{Y.ToString(format, provider)},{Z.ToString(format, provider)})";
    }
    #endregion
}

CodePudding user response：

The problem with using a private setter for an object is that it only prevents you from replacing the object entirely. As it's not an immutable object, you can still access its properties change them, as you have found.

You could define an IMyVector interface with get only properties, have MyVector implement it, and then use the interface for your public Position property.

public interface IMyVector
{
   float x {get;}
   ...
}

public class MyVector : IMyVector
{
...
}

public class Furniture : IMovable
{
   public string Name { get; private set; }
   public IMyVector Position { get; private set; }
   ...

CodePudding user response：

Another design possibility is to declare IMyReadOnlyVector interface and expose it whenever we don't want to allow change vectors:

public interface IMyReadOnlyVector {
  float x { get; }
  float y { get; }
  float z { get; }
}

public interface IMyVector : IMyReadOnlyVector {
  float x { get; set; }
  float y { get; set; }
  float z { get; set; }
}

Then you implement MyVector:

public class MyVector : IMyVector {
  public MyVector(float x, float y, float z) {
    this.x = x;
    this.y = y;
    this.z = z;
  }

  public float x { get; set; }
  public float y { get; set; }
  public float z { get; set; }
}

Now, time for the trick: IMovable uses IMyReadOnlyVector interface: we let user see Position but not allow to change it.

public interface IMovable {
  string Name { get; }
  // User can see position, but not allowed to change it
  IMyReadOnlyVector Position { get; }
}

public class Furniture : IMovable {
  // Private usage only: we don't want user explicitly change position
  private MyVector m_Position;

  public string Name { get; private set; }

  // Public usage: user can't change vector's coordinates here
  public IMyReadOnlyVector Position => m_Position;

  public Furniture(string name, float x, float y, float z) {
    this.Name = name;
    this.m_Position = new MyVector(x, y, z);
  }

  // But we can change Position within the class 
  public void ShiftMe(int dx, int dy, int dz) {
    m_Position.x  = dx;
    m_Position.y  = dy;
    m_Position.z  = dz;
  }
}