Skip to content

Creates a generic Maze Drawer class#9224

Merged
Hamms merged 3 commits into
stagingfrom
create-generic-maze-Drawer-class
Jul 5, 2016
Merged

Creates a generic Maze Drawer class#9224
Hamms merged 3 commits into
stagingfrom
create-generic-maze-Drawer-class

Conversation

@Hamms

@Hamms Hamms commented Jun 28, 2016

Copy link
Copy Markdown
Contributor

And refactors BeeItemDrawer and DirtDrawer to extend it.

Previously, BeeItemDrawer inherited from DirtDrawer, although DirtDrawer
contained a lot of functionality that BeeItemDrawer intentionally
discarded. This refactor plus ES6 class implementation makes their
shared vs individual functionality a bit cleaner, and will also make it
easier to add a new Drawer for the new WIP Collector Level type.

And refactors BeeItemDrawer and DirtDrawer to extend it.

Previously, BeeItemDrawer inherited from DirtDrawer, although DirtDrawer
contained a lot of functionality that BeeItemDrawer intentionally
discarded. This refactor plus ES6 class implementation makes their
shared vs individual functionality a bit cleaner, and will also make it
easier to add a new Drawer for the new WIP Collector Level type.
@Hamms

Hamms commented Jun 28, 2016

Copy link
Copy Markdown
Contributor Author

I would highly recommend reviewing this PR with whitespace ignored

@islemaster

Copy link
Copy Markdown
Contributor

This LGTM. I'm a little surprised this change adds more code than it removes :) but it's a nice refactor. 👍

@islemaster

Copy link
Copy Markdown
Contributor

Looks like some tests depended on something you removed?

Comment thread apps/src/maze/beeItemDrawer.js Outdated
import Drawer from './drawer';
const SQUARE_SIZE = 50;
let SVG_NS = require('../constants').SVG_NS;
let cellId = require('./mazeUtils').cellId;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const or import?

@Hamms Hamms force-pushed the create-generic-maze-Drawer-class branch from 897aab9 to 4d8d50f Compare June 30, 2016 21:05
@Hamms

Hamms commented Jul 5, 2016

Copy link
Copy Markdown
Contributor Author

PTA(quick)L

Comment thread apps/src/maze/dirtDrawer.js Outdated
* @return {number}
*/
spriteIndexForDirt_(val) {
static spriteIndexForDirt_(val) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice... I think this is the first time I've seen someone use the static keyword in JavaScript.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's exciting to see it added to the language! JavaScript feels like it's growing up ... 😂

@islemaster

Copy link
Copy Markdown
Contributor

LGTM

@Hamms Hamms merged commit 648b117 into staging Jul 5, 2016
@Hamms Hamms deleted the create-generic-maze-Drawer-class branch August 9, 2016 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants