I made a little drawing program in OOP.
The game consists of a canvas which is basically a 2d array where the user can draw various shapes on it. There can be multiple shapes overlapped on top of each other at the same spot. The shape that was drawn the last would appear on the top of the canvas and that is what gets printed out when we print the canvas - only those shapes at the top would be printed out.
Here I have two classes Rectangle
and Canvas
class Rectangle {
constructor(positions) {
this.positions = positions
this.character = Symbol()
}
move(dx, dy) {
for (const position of this.positions) {
const nx = position[0] + dx
const ny = position[1] + dy
position[0] = nx
position[1] = ny
}
}
}
class Canvas {
constructor(size) {
this.canvas = Array.from({ length: size }, () =>
Array.from({ length: size }, () => null)
)
}
draw(rectangle) {
const { positions, character } = rectangle
for (const [row, col] of positions) {
if (this.canvas[row][col] === null) this.canvas[row][col] = [character]
else this.canvas[row][col].push(character)
}
}
remove(rectangle) {
const { positions, character } = rectangle
if (positions.length === 0) return
for (const position of positions) {
const list = this.canvas[position[0]][position[1]]
list.splice(list.indexOf(character), 1)
}
}
print(palette) {
let rows = ''
for (let i = 0; i < this.canvas.length; i++) {
for (let j = 0; j < this.canvas[i].length; j++) {
if (this.canvas[i][j] === null || this.canvas[i][j].length === 0) {
rows += '*' // empty
} else {
rows += palette[this.canvas[i][j][this.canvas[i][j].length - 1]]
}
}
rows += '\n'
}
console.log(rows)
}
moveToTop(rectangle) {
this.remove(rectangle)
this.draw(rectangle)
}
drag(startPosition, endPosition, map) {
const list = this.canvas[startPosition[0]][startPosition[1]]
if (list?.length === 0) return
const character = list[list.length - 1]
const rectangle = map[character]
const dx = endPosition[0] - startPosition[0]
const dy = endPosition[1] - startPosition[1]
this.remove(rectangle)
rectangle.move(dx, dy)
this.draw(rectangle)
}
}
And I have a map that maps the Rectangle
's character
to that Rectangle instance. and another object called palette
which tells the canvas how that shape or pixel would look like when being printed (initially palette
is part of canvas
but I feel like I should decouple them so canvas
only holds the state of the current canvas and is not opinionated on how the shapes should look like when they are printed out).
const rectangle1 = new Rectangle(
[
[0, 1],
[1, 2],
[0, 2],
]
)
const rectangle2 = new Rectangle(
[
[0, 0],
[0, 1],
[1, 0],
]
)
const rectangle3 = new Rectangle([[0, 0]])
const palette = {
[rectangle1.character]: 'A',
[rectangle2.character]: 'B',
[rectangle3.character]: 'C',
}
const map = {
[rectangle1.character]: rectangle1,
[rectangle2.character]: rectangle2,
[rectangle3.character]: rectangle3,
}
here is the link to a live demo
Please feel free to give me any feedback. Specifically, I would love to know:
- if there is a better to approach the OO design here? Did I do a good job at separating the concerns here? Does every class have the methods here?
- I think I need some help to refactor the
map
andpalette
. Specificallymap
, it is manually constructed right now. I am not sure if I need to create two classes for them. - Is there any alternatives to approach implementing such a canvas? What are some of the pros and cons?
1 Answer 1
Question responses
if there is a better to approach the OO design here?
It seems okay. As was pointed out in answers to your other recent questions there currently is no use of OO features like inheritance.
Did I do a good job at separating the concerns here?
The separation seems okay.
Suggestions
Simplify move
method
The Rectangle
method move
is currently implemented as this:
move(dx, dy) { for (const position of this.positions) { const nx = position[0] + dx const ny = position[1] + dy position[0] = nx position[1] = ny } }
Variables nx
and ny
are only used once. The method could greatly be simplified:
position[0] += dx;
position[1] += dy;
functional approach to print
In the Canvas
method print
two for
loops are used. A functional approach, similar to how the canvas is created in the constructor, would allow fewer lines and the variable rows
could be assigned in one statement, which would allow const
to be used, though this would not be wise to change if performance is a major concern.
print(palette) {
const rows = this.canvas.map(column => column.map(cell => {
if (cell === null || !cell.length) {
return '*';
}
return palette[cell[cell.length - 1]];
}).join('') + "\n" ).join('');
check parameter types
Since JavaScript is loosely typed, there isn’t much support for ensuring a method is called with parameters of certain types. However, if a method expects to be called with certain types of arguments (e.g. Canvas::draw()
and ::remove()
expect a Rectangle
instance) then the type of those arguments can be checked - e.g. using the typeof
keyword. If an argument doesn’t match an expected type then perhaps an Error
should be throw
n.
Explore related questions
See similar questions with these tags.