Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal to make specifying nodes less redundant #38

Closed
AjaiKN opened this issue Mar 16, 2020 · 0 comments · Fixed by #44
Closed

Proposal to make specifying nodes less redundant #38

AjaiKN opened this issue Mar 16, 2020 · 0 comments · Fixed by #44

Comments

@AjaiKN
Copy link
Member

AjaiKN commented Mar 16, 2020

The problem

Currently, you have to use a different string for each node. For each side of a connection (and for each floor that a set of stairs connects) you have to use a different string. Here's a simple example.

const hallwayConnections = [
  ["hallway1_to_hallway2", "hallway2_to_hallway1"],
  ["hallway1_to_hallway3", "hallway3_to_hallway1"],
]

const hallways = [
  // hallway 1
  new Hallway([
    new Fork(BACK, "hallway1_to_hallway3", "the third hallway"),
    new Room("a"),
    new Fork(RIGHT, "hallway1_to_hallway2", "the second hallway"),
  ]),

  // hallway 2
  new Hallway([
    new Fork(FRONT, "hallway2_to_hallway1", "the first hallway"),
  ]),

  // hallway 3
  new Hallway([
    new Fork(LEFT, "hallway3_to_hallway1", "the first hallway"),
  ])
]

new Building(hallways, hallwayConnections)

It fells redundant that we should need to provide hallwayConnections, since it seems obvious that hallway1_to_hallway2 is connected to hallway2_to_hallway1.

Even worse in TypeScript

The example above isn't type safe in TypeScript. We can make it more type safe by using enums (like we're doing in walnut.direct) and adding a generic parameter to Building (like I was planning in #31):

enum ConnectionNode {
  HALLWAY1_TO_HALLWAY3,
  HALLWAY3_TO_HALLWAY1,
  HALLWAY1_TO_HALLWAY2,
  HALLWAY2_TO_HALLWAY1,
}

const hallwayConnections = [
  [ConnectionNode.HALLWAY1_TO_HALLWAY2, ConnectionNode.HALLWAY2_TO_HALLWAY1],
  [ConnectionNode.HALLWAY1_TO_HALLWAY3, ConnectionNode.HALLWAY3_TO_HALLWAY1],
]

const hallways = [
  // hallway 1
  new Hallway([
    new Fork(BACK, ConnectionNode.HALLWAY1_TO_HALLWAY3, "the third hallway"),
    new Room("a"),
    new Fork(RIGHT, ConnectionNode.HALLWAY1_TO_HALLWAY2, "the second hallway"),
  ]),

  // hallway 2
  new Hallway([
    new Fork(FRONT, ConnectionNode.HALLWAY2_TO_HALLWAY1, "the first hallway"),
  ]),

  // hallway 3
  new Hallway([
    new Fork (LEFT, ConnectionNode.HALLWAY3_TO_HALLWAY1, "the first hallway"),
  ])
]

new Building<ConnectionNode>(hallways, hallwayConnections)

Now, it's better typed, but it's even more redundant and confusing. And including stairs makes everything even more redundant.

Proposed solution

Fork example

Instead of using a different string for reverse connections, let's just provide a function called reverseConnection. Then, we don't even need to provide the hallwayConnections, since room-finder will be able to figure out what all the connections are.

const hallways = [
  // hallway 1
  new Hallway([
    new Fork(BACK, "hallway1_to_hallway3", "the third hallway"),
    new Room("a"),
    new Fork(RIGHT, "hallway1_to_hallway2", "the second hallway"),
  ]),

  // hallway 2
  new Hallway([
    new Fork(FRONT, reverseConnection("hallway1_to_hallway2"), "the first hallway"),
  ]),

  // hallway 3
  new Hallway([
    new Fork(LEFT, reverseConnection("hallway1_to_hallway3"), "the first hallway"),
  ])
]

new Building(hallways)

TypeScript example

When using TypeScript, you can just can use a string union as a type parameter to make things more type safe. (You could also use an enum.)

const hallways = /*... same as above ... */

type MyConnections = "hallway1_to_hallway2" | "hallway1_to_hallway3"

new Building<MyConnections>(hallways)

Stairs example

For dealing with stairs, we could provide a function called onFloor.

const hallways = [
  // hallway 1 (on 1st floor)
  new Hallway([
    new Stairs(LEFT, onFloor("stair-a", 1)),
    new Room("a"),
  ]),

  // hallway 4 (on 2nd floor)
  new Hallway([
    new Stairs(LEFT, onFloor("stair-a", 2)), 
  ])
]

new Building(hallways)

Mentions

@presssssure @bholmbertelsen @tghuman Since you all have used room-finder at least a little, what do you think about this? I want to do this before implementing the accessibility API (#30), since it'll have an effect on what the accessibility API looks like.

@AjaiKN AjaiKN mentioned this issue Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant