restricting access to a public setter

Question

My question is how I can achieve more encapsulation in TypeScript.

I have a class Item, with a public setter isOwned, but I only want to call this method in specific situations: if the item is picked up or dropped by a Player.

/** @package A */
class Item {
    private _is_owned: boolean = false

    // ...

    /** @returns - Has this item been picked up by a player? */
    get isOwned(): boolean {
        return this._is_owned
    }

    /** @param owned - Is a player picking up this item? */
    set isOwned(owned: boolean) {
        this._is_owned = owned
    }
}

/** @package B */
class Player {
    private _items: Set<Item> = new Set()

    // ...

    /**
     * @param item - the item in question
     * @returns - Does this player own the item?
     */
    owns(item: Item): boolean {
        return this._items.has(item)
    }

    /**
     * @param item - the item to pick up
     * @throws - if the item is already owned
     */
    pickUp(item: Item): void {
        if (item.isOwned) throw new Error('cannot pick up item that is already owned')
        this._items.add(item)
        item.isOwned = true // WARNING public method
    }

    /**
     * @param item - the item to drop
     * @throws - if this player does not own the item
     */
    drop(item: Item): void {
        if (!this.owns(item)) throw new Error('cannot drop item that player does not own')
        this._items.delete(item)
        item.isOwned = false // WARNING public method
    }
}

The methods Player#pickUp and Player#drop need to check the ownership of an item to determine whether the player is allowed to pick up or drop the item. So in those methods, the setter Item#isOwned is called, and therefore it needs to be public. (In TypeScript, there is no such thing as package-private, and even if there were, Player and Item are not in the same package.)

I want that setter to be called only in those methods though, because there are security implications: Since Item#isOwned is public, a “hacker” could bypass the checks before the #pickUp and #drop methods, as so:

/**
 * Allow a player to pick up an item,
 * even if that item is owned by another player.
 */
function hack(player1: Player, player2: Player, item: Item): void {
    player1.pickUp(item)
    try {
        player2.pickUp(item)
    } catch (e) {
        item.isOwned = false // <!-- this is very bad
        player2.pickUp(item)
    }
}

I’ve tried moving the methods to the Item class, but that won’t work because the methods need to access the private field Player#_items.

Then I tried separating each method into two: one in class Player where the item is added/deleted to the player, and another in class Item where ownership is tested, but that gives the same problem (just with different method names).

It seems like no matter what I do, getting/setting the ownership of an item needs to be public, so any program that calls Player#pickUp and Player#drop will also be able to get/set the item’s ownership status.

So how do I solve this problem? Is there a different data structure I need to use, or do I need to rethink my entire strategy?

Answer 1

For a little bit more control over the ownership of the item, you could store who owns the item instead of storing that the item is owned.

I'm not familiar with the syntax, so here's some pseudocode.

class Player
{
  List<item> items;

  public void Pickup(item)
  {
    var newItem = item.PickedUpBy(this);
    if(newItem != null)
    {
      world.replaceItem(item, newItem); // assuming you have a place were the original item came from
      items.add(newItem);
    }
  }

  public void Drop(item)
  {
    var newItem = item.DroppedBy(this);
    if(newItem != null)
    {
      items.remove(item);
      world.replaceItem(item, newItem);
    }
  }
}

class Item
{
  private Player owner;

  public Item PickedUpBy(Player player)
  {
    if(owner != null)
    {
      // Item is owned, cannot pickup
      return null;
    }
    Item newItem = new Item(....);
    newItem.owner = player;
    return newItem;
  }

  public Item DroppedBy(Player player)
  {
    if(player != owner)
    {
      // Throw hacker error
    }
    Item newItem = new Item(....);
    newItem.owner = null;
    return newItem;
  }
}

However, if a hacker can execute code, like in your example, this will not prevent them from manipulating objects in memory.

Answer 2

You can do this in C++ by declaring a friend class or even just a friend function, and in Swift by declaring a fileprivate setter which can only be used in that file.

In many languages you could have a setter with a second parameter saying who asks for the value to be changed, where you have to pass in the caller and the compiler does the type checking; the setter should check that the value passed is not nil. Of course a caller can get around this if it has a reference to any object of the right class, but then nothing is bullet proof.

Answer 3

If the decision that something is "owned" is based on whether some external object holds a reference to it, then I think a public boolean setter represents very different semantics than what you intend.

Instead, I'd consider a method like "TakeOwnership(parent)" and use it to store a reference. Then remove the public setter, and simply have business logic in your IsOwned() getter that determines whether the parent reference is still valid.

Getters don't have to represent a field; they can contain business logic; that's part of the value of abstracting them as a getter instead of publicly exposing the field.

One of the tradeoffs in exposing public setters is that, while convenient for initially populating values, you risk accidental complexity due to the Law of Demeter. If allowing external code to change your state isn't what you want, you don't need to.

An alternative, depending on how your object lifetimes end up working, might be to store a reference to a OwnershipRoster, which could contain mappings between objects capable of owning things and the things that they own, and defer IsOwned implementation to that.