patternjavascriptMinor
Using generators to build up a maze
Viewed 0 times
mazebuildgeneratorsusing
Problem
In prepping to teach a workshop on recursion I wrote this code that uses a maze-building algorithm. In doing so I found it really natural to use generators a lot. I feel pretty happy with how the code turned out but its also pretty unusual and I wonder what yall think of it.
This is meant to run on latest chrome (partial es2015 support, no destructing assignment, no modules yet)
board.move(fromCell, toCell)
yield
yield* createMazeOnUnvisitedNeighbors(board, toCell)
}
function* createMazeOnUnvisitedNeighbors(board, cell) {
const surroundingCells = board.surroundingCells(cel
This is meant to run on latest chrome (partial es2015 support, no destructing assignment, no modules yet)
* {
box-sizing: border-box;
}
body {
background-color: red;
min-height: 100vh;
padding: 0;
margin: 0;
display: flex;
}
main {
flex: 1;
display: flex;
flex-direction: column;
}
.row {
display: flex;
flex: 1;
}
.row:first-child .cell {
border-top: 2px solid red;
}
.row .cell:first-child {
border-left: 2px solid red;
}
.cell {
display: flex;
background-color: yellow;
width: 2%;
border-bottom: 2px solid red;
border-right: 2px solid red;
transition: background-color 1.5s;
}
.cell.clear-right {
border-right: 0;
}
.cell.clear-down {
border-bottom: 0;
}
.cell.visited {
background-color: grey;
}
fieldset {
position: fixed;
bottom: 0;
right: 0;
background-color: beige;
max-height: 5px;
transition: max-height .25s;
overflow: hidden;
}
fieldset:hover {
max-height: 3em;
}
Speed:
Finish
(function(){
'use strict'
var speed = document.querySelector('[name=speed]')
var runLater = fn => setTimeout(fn, speed.value)
document.querySelector('[name=finish]').addEventListener('click', () => runLater = fn => fn())
const board = createBoard(50, 50)
const cell = pos(randomInteger(board.width), randomInteger(board.height))
run(createMaze(board, null, cell))
function run(iterator, prev) {
if( (prev||{}).done ) return
runLater( () => run(iterator, iterator.next()))
}
function* createMaze(board, fromCell, toCell) {
// console.log(moving from ${fromCell} -> ${toCell}`)board.move(fromCell, toCell)
yield
yield* createMazeOnUnvisitedNeighbors(board, toCell)
}
function* createMazeOnUnvisitedNeighbors(board, cell) {
const surroundingCells = board.surroundingCells(cel
Solution
Indentation
You're missing a level of indentation in your IIFE - it should be:
I've used spaces rather than a tab as I prefer that. I'm not going to recommend one over the other as I don't want to start a holy war.
Spacing
You're inconsistent with spacing around your brackets, sometimes they get a space and sometimes they don't:
vs
I'd generally never put a space after an opening bracket or before a closing one.
That's the stylistic stuff out the way, I haven't mentioned semicolons because I don't want to start the other JavaScript holy war.
Hoisting
No, hoisting isn't good. It confuses beginners and I don't like having to scroll below the line I'm reading to find the function. Declare your functions first.
Ask some beginners what the code above logs. I bet at least some of them will get it wrong. Obviously naming a variable the same as a function in the same scope is a pretty big mistake but I have seen it happen.
Confusing code
Look at this bit of code:
First,
I'd move all of the
Recursion
Tail recursion is a special case that is optimised in many languages/compilers to a loop. A loop is far simpler as you don't need to save a new frame on the stack.
If you refactor that to use a loop you can also only compute the cells' html once instead of having to recompute it for each row (width doesn't change between rows).
I also think all the generator/iterator stuff is pointless for the html as it is all called from
File outline
I see what you're saying about being able to read the file as an outline, I generally do this sort of thing:
You're missing a level of indentation in your IIFE - it should be:
(function() {
'use strict'I've used spaces rather than a tab as I prefer that. I'm not going to recommend one over the other as I don't want to start a holy war.
Spacing
You're inconsistent with spacing around your brackets, sometimes they get a space and sometimes they don't:
board.surroundingCells(cell)vs
surroundingCells.filter(c => !board.isVisited(c) )I'd generally never put a space after an opening bracket or before a closing one.
That's the stylistic stuff out the way, I haven't mentioned semicolons because I don't want to start the other JavaScript holy war.
Hoisting
No, hoisting isn't good. It confuses beginners and I don't like having to scroll below the line I'm reading to find the function. Declare your functions first.
var a = true;
function a() {
return "hello";
}
console.log(typeof a);Ask some beginners what the code above logs. I bet at least some of them will get it wrong. Obviously naming a variable the same as a function in the same scope is a pretty big mistake but I have seen it happen.
Confusing code
Look at this bit of code:
var speed = document.querySelector('[name=speed]')
var runLater = fn => setTimeout(fn, speed.value)
document.querySelector('[name=finish]').addEventListener('click', () => runLater = fn => fn())
function run(iterator, prev) {
if( (prev||{}).done ) return
runLater( () => run(iterator, iterator.next()))
}First,
speed is a poor name as it is actually a delay in milliseconds. prev is also a poor name, I assume you meant previous but even then, I think current is a better name. It's certainly not the previous step.I'd move all of the
run stuff into a separate object with functions like start, changeDelay and finish.Recursion
Tail recursion is a special case that is optimised in many languages/compilers to a loop. A loop is far simpler as you don't need to save a new frame on the stack.
function* getRowsHtml(height, width) {
if(height '
yield* getCellsHtml(width)
yield ''
yield* getRowsHtml(height-1, width)
}If you refactor that to use a loop you can also only compute the cells' html once instead of having to recompute it for each row (width doesn't change between rows).
I also think all the generator/iterator stuff is pointless for the html as it is all called from
getTableHtml which is only called by Array.from.File outline
I see what you're saying about being able to read the file as an outline, I generally do this sort of thing:
(function (w, $) {
'use strict';
var field1,
field2,
function1,
main;
function1 = function(arg) {
// Do something important.
};
main = function() {
// entry point.
};
main();
}(window, jQuery));Code Snippets
(function() {
'use strict'board.surroundingCells(cell)surroundingCells.filter(c => !board.isVisited(c) )var a = true;
function a() {
return "hello";
}
console.log(typeof a);var speed = document.querySelector('[name=speed]')
var runLater = fn => setTimeout(fn, speed.value)
document.querySelector('[name=finish]').addEventListener('click', () => runLater = fn => fn())
function run(iterator, prev) {
if( (prev||{}).done ) return
runLater( () => run(iterator, iterator.next()))
}Context
StackExchange Code Review Q#110994, answer score: 4
Revisions (0)
No revisions yet.