marijnh / Eloquent-JavaScript

The sources for the Eloquent JavaScript book
https://eloquentjavascript.net
3.01k stars 793 forks source link

Chapter 11 - Message Routing - Needn't catch when connections not defined ? #460

Closed MaddyGit closed 5 years ago

MaddyGit commented 5 years ago

Line mentioned in the comment in requestType() method refers to connections in nest.state which might not be defiend at times, like upon very first call to everywhere() in this section of code:

  1. everywhere() defined connections for first Node, say [Church Tower] and called broadcastConnections( [Church Tower], 'Church Tower' )

  2. broadcastConnections() called request( [Church Tower], [Sportsgrounds], 'connections', { name: 'Church Tower', neighbors: [Array of neighbors of 'Church Tower'] } ) which called [Church Tower].send( [Sportsgrounds], 'connections', { name: 'Church Tower', neighbors: [neighbors of 'Church Tower'] }, resolverCallback ) which in turn reached handler-callback method defined in requestType() with arguments ( [Sportsgrounds], { name: 'Church Tower', neighbors: [neighbors of 'Church Tower'] }, 'Church Tower' )

  3. Now [Sportsgrounds] node doesn't yet have state.connections defined as it would be defined in everywhere() method's (nth after first i.e. 2nd or 3rd or . . .) call and by now everywhere() 's first call hasn't returned yet, So the referencing to nest.state.connections (nest is [Sportsgrounds]) would throw TypeError: nest.state.connections is undefined shouldn't code handle this like by returning/defining when undefined ? "Here on my machine I was able to produce this error after plucking out setTimeout() s from the send() method."

requestType("connections", (nest, {name, neighbors},
                            source) => {
//Following is the line of code with issue
  let connections = nest.state.connections;
  if (JSON.stringify(connections.get(name)) ==
      JSON.stringify(neighbors)) return;
  connections.set(name, neighbors);
  broadcastConnections(nest, name, source);
});

function broadcastConnections(nest, name, exceptFor = null) {
  for (let neighbor of nest.neighbors) {
    if (neighbor == exceptFor) continue;
    request(nest, neighbor, "connections", {
      name,
      neighbors: nest.state.connections.get(name)
    });
  }
}

everywhere(nest => {
  nest.state.connections = new Map;
  nest.state.connections.set(nest.name, nest.neighbors);
  broadcastConnections(nest, nest.name);
});

Reference to the Code

marijnh commented 5 years ago

Hi. Since sending messages between nodes is asynchronous, and everywhere runs synchronously on every nest, all nodes will have a value in state.connections when they start receiving "connections" messages. I agree this is a bit subtle, and the way everywhere works isn't obvious, but I don't think there's a bug in the current code.

MaddyGit commented 5 years ago

you're right, I didn't mean a bug, I was just worried about depending upon setTimeout as I hear it's not precise, still I'm new to JavaScript so you know better + can't thank you enough ever for compiling this book!