0

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?

  • 1
    "I want that setter to be called only in those methods though, because there are security implications" - You should not confuse encapsulation and security. They are completely orthogonal concepts. Making something private or public does not affect security in any way that matters. – Christian Hackl Feb 18 at 14:43
0

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.

  • “if a hacker can execute code” - ideally, a hacker couldn’t execute code, but it’s more of a complaint that the code is permissible. e.g. nothing to prevent my future self from executing it. – chharvey Jan 25 at 16:52
  • i'm confused by what your code does. could you please add some comments? it looks like every time a player picks up an item, a whole new item is constructed? – chharvey Jan 25 at 16:56
  • Correct, that concept is called immutability. I'd gladly explain other parts, anything specific? – Rik D Jan 25 at 17:44
  • i'm not sure that fits my model. say i create a new item, spoon, and at first it has no owner. then player1.pickUp(spoon) should give it an owner, to prevent player2 from picking it up. in your code though, a new spoon is created every time, so you could have an unlimited number of players picking up the spoon … not the exact same spoon, but an identical spoon. in my model, there is only one spoon (unless we're in the Matrix, in which case there is no spoon). any new spoons would need to be constructed separately. – chharvey Jan 25 at 18:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.