LOZORD / xanadu

A game of precarious alliances and breaking fast on honeydew.
Other
21 stars 12 forks source link

Create a Grid interface #50

Open LOZORD opened 8 years ago

LOZORD commented 8 years ago

This would be a generic interface (Grid<T>) that would be used instead of T[][]. It would allow for better abstraction and code de-specialization/clumping/copying.

jimbuck commented 8 years ago

You mind if I tackle this one? I've worked with grids before, both as mazes and terrain. I will can keep it faithful to the original interface design, or add any additional features (location validation, neighbor navigation, etc).

LOZORD commented 8 years ago

Yes please, that would be great! 🎃

jimbuck commented 8 years ago

Awesome! I just got the game up and running. There were 3 tslint errors, but they were all very easy to fix.

LOZORD commented 8 years ago

Hmm, really? Would you mind sharing the error output if you can?

jimbuck commented 8 years ago

image

LOZORD commented 8 years ago

Ah I see. I think the $(npm bin) is resolving to your global tslint instead of the local one (in node_modules), which may end up using a different tslint.json. Anyway, an error's and error, so thanks for the fix!

jimbuck commented 8 years ago

I though that may be the case, so I removed the $(npm bin) references in the npm scripts. Since it will use your node_modules folder by default, that would force it to use the correct version. But the errors were still found. I'm not sure how they got past the CI build though.

LOZORD commented 8 years ago

Yes please! That would be great 😁

On Wednesday, October 26, 2016, Jim Buck notifications@github.com wrote:

You mind if I tackle this one? I've worked with grids before, both as mazes https://embed.plnkr.co/Jwrjr9jJe6mmCDhceCde/ and terrain https://plnkr.co/edit/JmwDrTqgucpzpDO4lELY. I will can keep it faithful to the original interface design, or add any additional features (location validation, neighbor navigation, etc).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/LOZORD/xanadu/issues/50#issuecomment-256446138, or mute the thread https://github.com/notifications/unsubscribe-auth/ADhFUZdNHhsCtMyHfC8e2yw6_Tf_53G0ks5q36QxgaJpZM4J5n4d .

jimbuck commented 8 years ago

I've noticed a common pattern like the following:

export interface MyEntity {
  name: string;
  coolLevel: number;
}

export function isCool(entity: MyEntity): boolean {
  return entity.coolLevel > 5;
}

// Example:
let jim: MyEntity = {
  name: 'Jim',
  coolLevel: 1.9
};

isCool(jim) // returns false

Was this a chosen design pattern? Should that continue with the Grid implementation you're looking for? Any reason why you'd like to avoid something like this:

export class MyEntity {
  private name: string;
  private coolLevel: number;

  constructor({name: string, coolLevel: number}){
      this.name = name;
      this.coolLevel = coolLevel;
  }

  isCool(): boolean {
    return this.coolLevel > 5;
  }
}

// Example:
let jim = new MyEntity({
  name: 'Jim',
  coolLevel: 1.9
});

jim.isCool() // returns false
LOZORD commented 8 years ago

Well, @zthomae and I are both FP/Haskell nerds (see #28), so we've tended to shy away from using class or that kind of OOP in this project. If you want to match "our" style, that would be great; but if you want to do something like class XanaduGrid implements Grid that is ok, too. As you can see, I've used classes for the Game, Lobby, and Server. As long as it's tested and intuitive, whatever works.

jimbuck commented 8 years ago

I will definitely keep it functional (like the rest of the source). I usually stay classically OO, so it's nice to learn different design patterns!

Now one question though, strictly for educational purposes, regarding immutability and FP. In the past I've created specific implementation of immutable classes. For example (albeit overly simple):

// List of items where new items are add to the middle...
class MiddleList<T> {
  private _items: T[];

  public get count(): number {
    return this._items.length;
  }

  constructor(items: T[]) {
    this._items = items || [];
  }

  public add(item: T): MiddleList<T> {
    let index = Math.round(this.count / 2);
    let items = [...this._items];
    items.splice(index, 0, item);

    return new MiddleList(items);
  }

  public remove(item: T): MiddleList<T> {
    let index = this._items.indexOf(item);
    let items = [...this._items];
    if (index > -1) {
      items.splice(index, 1);
    }

    return new MiddleList(items);
  }

  public toString(): string {
    return '[' + this._items.toString() + ']';
  }
}

What are your thoughts on that? Is that a useful approach for more complex structures like grids, etc? In your experience would there be any drawbacks or pitfalls to this approach? Is it better to just use the built in structures from a library like immutable.js? Thank you for your insight!

LOZORD commented 8 years ago

For now, you don't need to use Immutable.js if you don't want. There's also no strict immutability standard across the project, as you can probably tell. Basically, the purpose for the Grid interface is to cut down on the potential copying of code and nitty-gritty cell accessing (which I've messed up before in this project). For some "historical" context, the project used the fixed-2d-array package for a while, but after finding some gnarly bugs in it, I switched to a "suboptimal" T[][] interface. This is basically a return to having an interface instead of just working with "raw" 2D arrays.