Skip to content

Turtle graphics #2

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.

Sign up for GitHub

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

Open
David-Ko wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Turtle graphics #2

Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
27a88d8
Create box_it_script_stretch project
David-Ko Nov 15, 2018
d456725
Created and commited first version of turtle.js
David-Ko Nov 20, 2018
7333aa6
Added print() method to complete my code
David-Ko Nov 23, 2018
c1f24b6
Try to commit final version of turtle.js to turle_graphics branch
David-Ko Nov 23, 2018
0c5264d
Corrected typo in line 135
David-Ko Nov 23, 2018
90088b5
deleted box_it_script.js
David-Ko Nov 24, 2018
5e52dc7
Combine line 8 and 9
David-Ko Dec 22, 2018
cca868f
Clean up white spaces and implemented chaining
David-Ko Dec 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions turtle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
// worked on it on Monday Nov 19, 2018 at 930pm
class Turtle {

constructor (x, y){
Copy link

@maxwellgordon maxwellgordon Nov 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

when defining a function, please be consistent with your spacing. I notice here you use the following:

functionName (param1, param2){

while on line 13 you do

functionName(){

and on 20 you do

functionName() {

I prefer the following spacing:

function functionName(param1, param2) {

(the function keyword can be omitted when the method is defined within a class)

This consistency is important for potential employers looking at your code. It shows attention to detail. Similarly, so does consistently with using (or not using) ;. To be inconsistent with either is akin to having typos in your resume. It immediately sends the wrong first impression.

Again, I prefer to always use semi-colons after every line that needs them.

Copy link
Owner Author

@David-Ko David-Ko Dec 22, 2018

Choose a reason for hiding this comment

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

Thanks Max. Changes made.

this.x = x;
this.y = y;
this.face = 'east';
this.coordinates = [];
this.coordinates.push([this.x, this.y])
Copy link

@maxwellgordon maxwellgordon Nov 28, 2018

Choose a reason for hiding this comment

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

Since you are initializing this.coordinates here, you can set it's initial value to [this.x, this.y] as opposed to doing in two steps, like you did on lines 8 and 9

this.coordinates = [this.x, this.y];

Copy link
Owner Author

@David-Ko David-Ko Dec 22, 2018

Choose a reason for hiding this comment

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

THanks Max. Changes made.


}

allPoints(){

Copy link

@maxwellgordon maxwellgordon Nov 28, 2018

Choose a reason for hiding this comment

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

try to avoid extra whitespace within your functions

Copy link
Owner Author

@David-Ko David-Ko Dec 22, 2018

Choose a reason for hiding this comment

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

Thanks Max. Changes made.

return this.coordinates

}

forward(steps) {

if (this.face === 'east'){
for (let i = 0; i < steps; i++) {
this.x += 1;
this.coordinates.push([this.x, this.y])
}
} else if (this.face === 'south') {
for (let i = 0; i < steps; i++) {
this.y += 1;
this.coordinates.push([this.x, this.y])
}
} else if (this.face === 'west') {
for (let i = 0; i < steps; i++) {
this.x += -1;
this.coordinates.push([this.x, this.y])
}
} else if (this.face === 'north') {
for (let i = 0; i < steps; i++) {
this.y += -1
this.coordinates.push([this.x, this.y])
}
}
}
right(){
if (this.face === 'east') {
this.face = 'south'
} else if (this.face === 'south') {
this.face = 'west'
} else if (this.face === 'west') {
this.face = 'north'
} else if (this.face === 'north') {
this.face = 'east'
}
}

left(){
if (this.face === 'east') {
Copy link

@maxwellgordon maxwellgordon Nov 28, 2018

Choose a reason for hiding this comment

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

this would be an excellent place to use a switch statement!

left() {
    switch(this.face) {
        case 'east':
            this.face = 'north';
            break;
        case 'north':
            this.face = 'west';
            break;
        case 'west':
            this.face = 'south';
            break;
        case 'south':
            this.face = 'east';
            break;
    }
}

this.face = 'north'
} else if (this.face === 'north') {
this.face = 'west'
} else if (this.face === 'west') {
this.face = 'south'
} else if (this.face === 'south') {
this.face = 'east'
}
}

print(){
let allCoordinates = this.coordinates;
let maxCoordinates = [];
let board = [];
let boardString = '';

function findMax (x){
Copy link

@maxwellgordon maxwellgordon Nov 28, 2018

Choose a reason for hiding this comment

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

It is helpful to give your variables names that are descriptive.
Here x is confusing. With a name like x, given the context of the problem, it is easy for one to assume it has something to do with an x-coordinate, especially since we are using findMax to the find the maximum x- and y-coordinates within the array allCoordinates.
A better, more descriptive name for the parameter would have been coordinateArray or something like that

Copy link

@maxwellgordon maxwellgordon Nov 28, 2018

Choose a reason for hiding this comment

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

Similarly, findMax could have been named a little better to something like findMaxXY or findMaxXYCoordinates which one would assume would return a value like

return [biggestX, biggestY];

And then you would use it as follows on line 90:

const maxCoords = findMaxXYCoordinates(allPoints);

and not you could use maxCoords within your print and get rid of your maxCoordinates array of length 1.

let biggestX = 0
let biggestY = 0
for (let i = 0; i < x.length; i++){
if(x[i][0] > biggestX){
biggestX = x[i][0]
}
}
for (let j = 0; j < x.length; j++){
if(x[j][1] > biggestY){
biggestY = x[j][1]
}
}

maxCoordinates.push([biggestX, biggestY])

}
findMax(allCoordinates)
// return maxCoordinates


function makeBoard(width, height){
for (let i = 0; i<=height; i++){
for (let j = 0; j<=width; j++){
board.push([j, i])
}
}
}
makeBoard(maxCoordinates[0][0], maxCoordinates[0][1])
// return board
function printBoard(board, width){
// console.log(board.length)
for(let boardPoint = 0; boardPoint < board.length; boardPoint++ ){
let currentPoint = board[boardPoint];
let isTurtleStep = allCoordinates.find((step) => {
// console.log(step[0], 'step')
// console.log(currentPoint[0], 'currentPoint')
return (step[0] === currentPoint[0]) && (step[1] === currentPoint[1])
})
if (currentPoint[0] === width){ // width - 1?
boardString += isTurtleStep ? '* \n' : '0 \n';
} else {
boardString += isTurtleStep ? '*' : '0';
}
}
// console.log(boardString)
Copy link

@maxwellgordon maxwellgordon Nov 28, 2018

Choose a reason for hiding this comment

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

when submitting your homework, same with when you would (later in your career) submit any PR, please remove any commented out lines of code.
You can always leave comments and they are encouraged (like you did on line 112. But please remove any commented out code (like on lines 108 and 109)

Copy link
Owner Author

@David-Ko David-Ko Dec 22, 2018

Choose a reason for hiding this comment

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

Thanks Max. Changes made.

}
printBoard(board, maxCoordinates[0][0])
console.log(boardString)
}

}

let Sonic = new Turtle (0,0)
Sonic
Sonic.forward(5)
Sonic.right()
Copy link

@maxwellgordon maxwellgordon Nov 28, 2018

Choose a reason for hiding this comment

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

Very good work around. I know you were unable to get the following to work

new Turtle(0, 0)
  .forward(5)
  .right()
  .forward(5)
  .right()
  .forward(5)
  .right()
  .forward(5)
  .print()

using chaining.

The reason this wasn't working was because you could only call right or forward (or any other method belonging to the turtle class) on an instance of a turtle.
I can see in your work around, that's exactly what you did. You realized that chaining wasn't working so you used your turtle instance (Sonic) to call the methods one at a time.

In order to get chaining to work, you need to make each of those methods described above (right, left, forward) have a return value.
Currently, since there is no explicit return statement, each method is implicitly returning undefined.
Because of this implicit return of undefined, when you chain a right onto a call to forward like below:

new Turtle (0,0)
    .forward(5)
    .right()

JS/Node thinks you are trying to invoke the right method on this implicitly returned undefined.
It can be thought of like this:

const turtle = new Turtle(0,0);
const turtle1 = turtle.forward(5); // turtle1 becomes undefined
turtle1.right()

So, what we would like is for each of those methods to return the turtle.
To do this, we need each method to return this at the end of it.
Like this:

 forward(steps) {
    // ...
     return this;
  }

 right() {
    // ...
     return this;
  }

 left() {
    // ...
     return this;
  }

And now chaining will work!

Copy link
Owner Author

@David-Ko David-Ko Dec 22, 2018

Choose a reason for hiding this comment

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

Thanks Max. Changes made. I can't believe i didn't think about that!

Sonic.forward(5)
Sonic.right()
Sonic.forward(5)
Sonic.right()
Sonic.forward(5)
Sonic.print()