-
Notifications
You must be signed in to change notification settings - Fork 0
BFS algorithm and path drawing added #3
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,15 @@ const Router = ReactRouter.BrowserRouter; | |
| const Route = ReactRouter.Route; | ||
| const Switch = ReactRouter.Switch; | ||
| const Game = require('./Game'); | ||
| const pathFind = require('../utils/BFS'); | ||
| // const Settings = require('./Settings'); | ||
| // const Results = require('./Results'); | ||
|
|
||
| function boardReset(lengthX, lengthY) { | ||
|
|
||
| console.log('creating board') | ||
| console.log('path?'); | ||
| console.log(pathFind); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't one console.log enough? |
||
| const board = []; | ||
| for (let i=0; i<lengthY; i++) { | ||
| board[i] = []; | ||
|
|
@@ -44,18 +48,23 @@ class App extends React.Component { | |
| this.handleClickOnBoard = this.handleClickOnBoard.bind(this); | ||
| this.handleStartClick = this.handleStartClick.bind(this); | ||
| this.handleEndClick = this.handleEndClick.bind(this); | ||
| this.handleGoClick = this.handleGoClick.bind(this); | ||
| this.updateStartLocation = this.updateStartLocation.bind(this); | ||
| this.updateEndLocation = this.updateEndLocation.bind(this); | ||
| this.updatePath = this.updatePath.bind(this); | ||
| } | ||
|
|
||
| handleClickOnBoard(x, y) { | ||
| console.log('trying to update state'); | ||
| const board = this.state.board; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use destructuring, everywhere: |
||
| if (this.state.startFlag) { | ||
| this.updateStartLocation(x, y); | ||
| } else if (this.state.endFlag) { | ||
| this.updateEndLocation(x, y); | ||
| } else { | ||
| const tempBoard = this.state.board; | ||
| const tempBoard = board.map((row) =>{ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't name things "temp". Even life is temporary, we all know that. Even "new" is better than "temp". |
||
| return row.map((cell)=>Object.assign({}, cell)); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shorter: return row.map(cell => ({ ...cell, }));But why clone the cell at all?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because I want to make changes to state only from |
||
| }) | ||
| tempBoard[y][x].allowed = this.state.board[y][x].allowed ? false : true; | ||
| console.log(tempBoard) | ||
| this.setState(() => { | ||
|
|
@@ -82,6 +91,28 @@ class App extends React.Component { | |
| }) | ||
| } | ||
|
|
||
| handleGoClick() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. handle/calculate/do/perform/compute are all meaningless. Your functions must start with a verb which is clear and end with a subject: "drawPath", "drawPathStep", "killBill", "brewCoffee".
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea what this function does without reading it. I only know when someone was planning to use it. |
||
| // console.log('go pressed!') | ||
| const {start, end, board} = this.state; | ||
| const boardCopy = board.map((row) =>{ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Second time this function appears here. Extract to function or method to DRY it. |
||
| return row.map((cell)=>Object.assign({}, cell)); | ||
| }) | ||
| const path = pathFind(boardCopy, start, end); | ||
| const step = -1; | ||
| const addSteps = (path, step) => { | ||
| if (path.length === 0) { | ||
| return | ||
| } | ||
| this.updatePath(path[0],step+1) | ||
| const timeoutForStepAddition = window.setTimeout(addSteps.bind(null, path.slice(1), step+1), 500) | ||
| } | ||
| addSteps(path, step); | ||
| // async path.map( (step, index) => { | ||
| // console.log('update'); | ||
| // await window.setTimeout(this.updatePath.bind(null,step,index), 500) | ||
| // }) | ||
| } | ||
|
|
||
| updateStartLocation(x, y) { | ||
| this.setState(() => { | ||
| return { | ||
|
|
@@ -100,6 +131,21 @@ class App extends React.Component { | |
| }) | ||
| } | ||
|
|
||
| updatePath(step, stepNum) { | ||
| const board = this.state.board; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Destructure |
||
| const tempBoard = board.map((row) =>{ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3rd time same function |
||
| return row.map((cell)=>Object.assign({}, cell)); | ||
| }) | ||
| const [x, y] = step; | ||
| tempBoard[y][x].stepVisited = stepNum; | ||
|
|
||
| this.setState(() => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Arrow functions are good for brevity: this.setState(() => ({ board: tempBoard, }));Better: name your temp board 'board' and use ES2015 shorthand object literal this.setState(() => ({ board, }));Even better, specifically for React's setState, which can accept an object literal to merge with current state: this.setState({ board, }); |
||
| return { | ||
| board: tempBoard | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| render() { | ||
| return ( | ||
| <Router> | ||
|
|
@@ -112,7 +158,8 @@ class App extends React.Component { | |
| handleClicks = {{ | ||
| handleClickOnBoard: this.handleClickOnBoard, | ||
| handleStartClick: this.handleStartClick, | ||
| handleEndClick: this.handleEndClick | ||
| handleEndClick: this.handleEndClick, | ||
| handleGoClick: this.handleGoClick | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Destructuring FTW: const { handleFoo, handleBar, } = this;
handleClicks = { handleFoo, handleBar, } |
||
| }} | ||
| updateLocations = {{ | ||
| updateStartLocation: this.updateStartLocation, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ function Cell(props) { | |
|
|
||
| const {x, y} = props.cellData; | ||
| props = props.props; | ||
| const {board, colorA, colorB, start, end} = props.state; | ||
| const {board, colorA, colorB, colorC, start, end} = props.state; | ||
| const lengthY = board.length; | ||
| // const styles = { | ||
| // cell: { | ||
|
|
@@ -20,7 +20,10 @@ function Cell(props) { | |
| <div className = 'cellShell' | ||
| style = {{'height': (40/lengthY) + 'vh', 'width': (40/lengthY) + 'vh'}}> | ||
| <div className = 'cell' | ||
| style = {{'backgroundColor': (board[y][x].allowed ? colorA : colorB)}} | ||
| style = {{'backgroundColor': | ||
| (board[y][x].stepVisited >= 0) ? colorC : (board[y][x].allowed ? colorA : colorB) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unreadable beyond even trying. You need to name things. |
||
|
|
||
| }} | ||
| onClick = {props.handleClickOnBoard.bind(null, x, y)} | ||
| > | ||
| {(start[0] === x) && (start[1] === y) && <div>start</div>} | ||
|
|
@@ -155,7 +158,7 @@ function GameButtonsPanel(props) { | |
| classes: 'btn success', | ||
| activated: () => {return false}, | ||
| disabled: () => {return (!disabledCheck(start) || !disabledCheck(end))}, | ||
| onClick: () => {}, | ||
| onClick: props.handleClickOnButtons.handleGoClick, | ||
| } | ||
| ]; | ||
| console.log('buttons ok') | ||
|
|
@@ -184,7 +187,8 @@ function GameScreen(props) { | |
| <GameButtonsPanel state = {props.state} | ||
| handleClickOnButtons = {{ | ||
| handleStartClick: props.handleClicks.handleStartClick, | ||
| handleEndClick: props.handleClicks.handleEndClick | ||
| handleEndClick: props.handleClicks.handleEndClick, | ||
| handleGoClick: props.handleClicks.handleGoClick | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Destructuring like in comment above |
||
| }} | ||
| /> | ||
| </div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| const pathFind = (field, startP, endP) => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
function findPath({ field, startPoint, endPoint, }){ |
||
| const path = []; | ||
|
|
||
| // tests if the point is inside the tested field | ||
| var inField = function (point) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't var |
||
| if (point[0]<field.length && point[0] >=0) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use destructuring to name all those unreadable indexes |
||
| if (point[1]<field[0].length && point[1] >=0) { | ||
| return true; | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your function can return undefined here. |
||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // tests if the point was already | ||
| var visited = function (point) { | ||
| if (field[point[0]][point[1]].stepVisited<0) { | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| // tests if a certain step is allowed | ||
| var allowedStep = function (point) { | ||
| if (field[point[1]][point[0]].allowed) { | ||
| return true; | ||
| } else if (!field[point[0]][point[1]].allowed) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| // tests if the tested point is in fact the ending point of the search | ||
| var isEndPoint = function(point) { | ||
| if (point === endP) { | ||
| return true; | ||
| } else { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't else after return |
||
| return false; | ||
| } | ||
| } | ||
|
|
||
| // returns the neighbours of a certain point | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comments don't help. Name your identifiers so you won't need comments: |
||
| var getNeighbours = function (point) { | ||
| var neighbours = []; | ||
| var movementX = [1, -1, 0, 0]; | ||
| var movementY = [0, 0, 1, -1]; | ||
| for (var i=0; i<4; i++) { | ||
| let neighbour = []; | ||
| neighbour[0] = point[0] + movementY[i]; | ||
| neighbour[1] = point[1] + movementX[i]; | ||
| neighbours.push(neighbour); | ||
| } | ||
| return neighbours; | ||
| } | ||
|
|
||
| // tests if a certain point can be used (inside the board && allowed to be stepped in && was not visited) | ||
| var legalPoint = function (point) { | ||
| if (inField(point)) { | ||
| if (allowedStep(point)) { | ||
| if (!visited(point)) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return isInField(point) && isAllowedStep(point) && !hasBeenVisited(point); |
||
| } | ||
|
|
||
| // returns the neighbours that are valid points to be used | ||
| var legalNeighbours = function (neighbours) { | ||
| var legal = []; | ||
| neighbours.forEach(function(neighbour) { | ||
| if (legalPoint(neighbour)) { | ||
| legal.push(neighbour); | ||
| } | ||
| }) | ||
| return legal; | ||
| } | ||
|
|
||
| // the BFS algorithm that searches for a path between the 'start' and 'end' points | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A link to an explanation is better |
||
| // returns 'true' if a path was found and 'false' if not | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant for a well named boolean function |
||
| var pathExists = function() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you copy/pasted if from somewhere, add a link. |
||
| if (legalPoint(startP) && legalPoint(endP)) { | ||
| var testingQue = [startP]; | ||
| field[testingQue[0][0]][testingQue[0][1]].stepVisited = 0; | ||
| while(testingQue.length>0) { | ||
| let currStep = testingQue[0]; | ||
| if (currStep[0] === endP[0] && currStep[1] === endP[1]) return true; | ||
|
|
||
| let currStepNum = field[currStep[0]][currStep[1]].stepVisited; | ||
| let allowedSteps = legalNeighbours(getNeighbours(currStep)); | ||
| allowedSteps.forEach(function(nextStep){ | ||
| field[nextStep[0]][nextStep[1]].stepVisited = currStepNum+1; | ||
| field[nextStep[0]][nextStep[1]].previous = currStep; | ||
| testingQue.push(nextStep); | ||
| }) | ||
| testingQue.shift(); | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| // a function that filles the 'path' object with pairs of key = step number : value = the tile location | ||
| var buildPath = function() { | ||
| var stepNum = field[endP[0]][endP[1]].stepVisited; | ||
| var currStep = endP; | ||
| path[0] = startP; | ||
| while (stepNum > 0) { | ||
| path[stepNum] = currStep; | ||
| stepNum--; | ||
| currStep=field[currStep[0]][currStep[1]].previous; | ||
| } | ||
| } | ||
|
|
||
| if (pathExists()) { | ||
| buildPath(); | ||
| // gameService.pathExistsFlag = true; | ||
| } | ||
|
|
||
| return path; | ||
|
|
||
| } | ||
|
|
||
| module.exports = pathFind; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import findPath from '../utils/BFS';There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I've been googling on the difference between
require()andimportand got into a wall of not understanding the difference between 'dynamic' and 'static' loading. And that's besides the fact that most places say thatimportis being transpiled by Babel torequire()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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
importeverywhere you can.