Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Feb 2, 2018

No description provided.

@shiffman
Copy link
Member

shiffman commented Feb 3, 2018

I appreciate this fix, but I think (even if backwards), the convention I'm using in the videos is i for rows and j for columns? Happy to re-open if I'm confused!

@shiffman shiffman closed this Feb 3, 2018
@ghost
Copy link
Author

ghost commented Feb 3, 2018

I actually did not do nothing to the code I just change the variables name ( "i" to "j" and "j" to "i").

your code before the "change" :

   for (let i = 0; i < this.rows; i++) {
        for (let j = 0; j < this.cols; j++) {
          this.data[i][j] += n.data[i][j];
        }
    }

the code after :

for (let j = 0; j < this.rows; j++) {
         for (let i = 0; i < this.cols; i++) {
           this.data[j][i] += n.data[j][i];
        }
    }

I did this "change" because I saw that no one is posting a pull request and I wanted that you will start the tutorial.

note : I'm 13 years old from Israel so my English is not very good. Love your videos by the way 👍
question : can you do a coding challenge about the game Tic Tac Toe?

@shiffman
Copy link
Member

shiffman commented Feb 3, 2018

Your English is great! Of course, now I get why you made the pull request, thank you again!

Thank you for writing and I love the idea of doing tic tac toe for sure!

@ghost ghost changed the title fixing add function (codingTrainLive)! submitting a pull requests in codingTrainLive! Feb 3, 2018
@ghost ghost changed the title submitting a pull requests in codingTrainLive! submitting a pull request in codingTrainLive! Feb 3, 2018
@ghost
Copy link
Author

ghost commented Feb 3, 2018

here is a few links that you can use if you will want to do the challenge:

thank's , Idan Turkenits from Israel :)

shiffman pushed a commit that referenced this pull request Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants