BFS algorithm and path drawing added #3
Conversation
| const Route = ReactRouter.Route; | ||
| const Switch = ReactRouter.Switch; | ||
| const Game = require('./Game'); | ||
| const pathFind = require('../utils/BFS'); |
There was a problem hiding this comment.
import findPath from '../utils/BFS';
There was a problem hiding this comment.
so I've been googling on the difference between require() and import and got into a wall of not understanding the difference between 'dynamic' and 'static' loading. And that's besides the fact that most places say that import is being transpiled by Babel to require() anyway because there is no native support yet... :/
-
Any chance for a link to clarify that difference between dynamic and static loading?
-
you marked only the
findPathto use withimport. Is there any reason not to useimporteverywhere instead of therequire()I've been using to for the other classes of the app? OrReactfor that matter?
There was a problem hiding this comment.
- It's a complex topic and I can write you the basics in chat.
- I only commented in this PR on changes, not on everything. If I'd comment on everything, we'd never finish. Gotta limit the scope, otherwise it's unmanageable. You should use
importeverywhere you can.
|
|
||
| console.log('creating board') | ||
| console.log('path?'); | ||
| console.log(pathFind); |
There was a problem hiding this comment.
Isn't one console.log enough?
|
|
||
| handleClickOnBoard(x, y) { | ||
| console.log('trying to update state'); | ||
| const board = this.state.board; |
There was a problem hiding this comment.
Use destructuring, everywhere: const { board, } = this.state;
| this.updateEndLocation(x, y); | ||
| } else { | ||
| const tempBoard = this.state.board; | ||
| const tempBoard = board.map((row) =>{ |
There was a problem hiding this comment.
Don't name things "temp". Even life is temporary, we all know that. Even "new" is better than "temp".
Name your mapping functions. This helps debugging and stack traces. Naming things in general helps you write better code.
| } else { | ||
| const tempBoard = this.state.board; | ||
| const tempBoard = board.map((row) =>{ | ||
| return row.map((cell)=>Object.assign({}, cell)); |
There was a problem hiding this comment.
Shorter:
return row.map(cell => ({ ...cell, }));But why clone the cell at all?
There was a problem hiding this comment.
because I want to make changes to state only from this.setState(), and since board is a nested array of objects, if i make a change to the cell's content it will change the state without invoking the setState function.
| } | ||
| } | ||
| } | ||
| return false; |
There was a problem hiding this comment.
return isInField(point) && isAllowedStep(point) && !hasBeenVisited(point);| return legal; | ||
| } | ||
|
|
||
| // the BFS algorithm that searches for a path between the 'start' and 'end' points |
There was a problem hiding this comment.
A link to an explanation is better
| } | ||
|
|
||
| // the BFS algorithm that searches for a path between the 'start' and 'end' points | ||
| // returns 'true' if a path was found and 'false' if not |
There was a problem hiding this comment.
Redundant for a well named boolean function
|
|
||
| // the BFS algorithm that searches for a path between the 'start' and 'end' points | ||
| // returns 'true' if a path was found and 'false' if not | ||
| var pathExists = function() { |
There was a problem hiding this comment.
If you copy/pasted if from somewhere, add a link.
|
|
||
| } | ||
|
|
||
| module.exports = pathFind; No newline at end of file |
There was a problem hiding this comment.
export default findPath
2 participants
No description provided.