Thanks Steve!

I watched your video and although I don't understand everything, I learned a lot. Separating my program into functions is a good idea. Your video clarified the map function for me.

Through your video, I understood that I was running code multiple times, when it only needs to run once (how I parsed my rules was one of these instances that you went over in your video), so thanks for pointing that out.

Again, thank you so much for taking the time to review my code. I really enjoyed your feedback.

> Again, thank you so much for taking the time to review my code. I really enjoyed your feedback.

Whatever you do, hold on to that. Never be mad about feedback just try your best to understand it, and if it's wrong explain yourself without getting frustrated, this is worth more than a dozen senior developers who explode at feedback (I would argue they're junior developers :) and you will have a wonderful career. Also look for good mentors, and don't be afraid to ask questions about direct feedback, if you didn't understand something he said just ask (this becomes an immediate learning opportunity and you will be better off for it), even if he doesn't get to answer, someone else might. This is a field where learning is endless, you will always learn something new as a software developer, new platforms are made all the time to be programmed on / with.

Having soft skills[0] will be the biggest asset you can have as a software developer, whether you do it for money or just for fun, if you have to collaborate in the open you will need to communicate with others. Nothing breaks a team like terrible communication and someone who rejects feedback out of pride.

[0]: https://en.wikipedia.org/wiki/Soft_skills

So glad to hear, Liam! Was a pleasure to put it together.

Good luck, and feel free to reach out if you get stuck on anything.

Every learner could do with a mentor like you!

This whole interaction is really reaffirming my faith in humanity. You guys are awesome!

Liam -- this response is just as impressive as the code that you wrote. You're obviously very smart, but that isn't good enough on its own. I've seen very-smart people who aren't open to critique, who get defensive rather than take the opportunity to learn new things. They don't get very far in life. On the other hand, the attitude you're showing here is tremendously valuable, and will serve you incredibly well throughout life. Never let yourself grow out of it!

If you would start a channel where you review code of random public repos I would definitely subscribe. Well done.

Drive-by code reviews (and maybe other types of annotated "audits", like for security bugs) would be a nice feature source repo sites might consider adding one day. It seems like a missing practice in open source development, code reviews (if any) are done by active developers once on check-in/merge acceptance. But as we see here code reviews can be a useful way to pay it forward too, not just new code / issue fixes, and done out-of-band there's no pressure to bow to the reviewer just to get something in; you can just have conversations and learn things.

The video format is a nice twist, in my distributed teams sometimes we've done "code review meetings" for some changes that are rather involved to capture some of the benefits of in-person / video code reviews.

Thanks for the kind words! Will definitely consider.

Watching your video was like watching Bob Ross help a wine and paint class. Well done.

Please consider. This is the rare time I'd actually use some sort of !remindme bot. Most lurking autodidacts, I assume, are hoping that you consider it too.

Come out of the woodwork, let him know! If I can, you can.

I would absolutely sub to this.

I'd love to both participate and lurk :)

me too i'll subscribe to your channel if it existed

Seconded. Do it. :-)

Let us know here if you are planning to do it :)

Just registered to +1 this

+1 Please and thank you

Ron Swanson?

+1 count me in

+1

+1, reminds me of Destroy All Software

Instead of this:

    (rule & (1 << i)) === 0 ? 0 : 1

You could just do:

    (rule >> i) & 1

Otherwise it was a good review! I'd probably also extract parts of this long line so it's not as cumbersome to read:

    return state.map((item, i) =>
        rules[((state[i - 1] || 0) * 4) + (item * 2) + (state[i + 1] || 0)]
    );

to

    return state.map((item, i) => {
        const prevItem = state[i - 1] || 0;
        const nextItem = state[i + 1] || 0;
        return rules[(prevItem << 2) | (item << 1) | nextItem];
    });

It also uses bitwise operations instead of arithmetic, even though they are equivalent in this context, simply because we are effectively operating on arrays of bits.

Thanks for the edits! I can't change the video, but did mention that both of those lines are a little clunky. I did update the gist though! Much cleaner.

I like to break expressions across multiple lines, and line up any patterns that are repeated, so your eye can easily scan up and down and obviously see what's the same, and what's different.

For example, instead of:

    var length = Math.sqrt((x * x) + (y * y));

You can go:

    var length = Math.sqrt((x * x) +
                           (y * y));

That way you can see the similarity of squaring x, and squaring y. But breaking up the multiplications because they repeat the x's and y's would be taking it too far -- it always requires balance and thinking about how can I communicate my intent and the SHAPE of the problem I'm solving.

You can also use indentation and formatting to bring out and make obvious other two-dimensional patterns and regularities, especially with two-dimensional cellular automata and graphics code.

There are some examples of that technique in my 2D cellular automata machine code. (There's a lot of code, so don't be overwhelmed, since I've been working on that since around 1985 or so. So never give up and just stick to it! You don't need to understand the code, which is pretty esoteric, just notice the patterns.)

https://github.com/SimHacker/CAM6/blob/master/javascript/CAM...

                        // Load the right two columns of the 3x3 window.
                        n  = cells[cellIndex - nextCol - nextRow];  ne = cells[cellIndex - nextRow];
                        c  = cells[cellIndex - nextCol          ];  e  = cells[cellIndex          ];
                        s  = cells[cellIndex - nextCol + nextRow];  se = cells[cellIndex + nextRow];

[...]

                            // Scroll the 3x3 window to the right, scrolling the middle and right
                            // columns to the left, then scooping up three new cells from the right
                            // leading edge.
                            nw = n;  n = ne;  ne = cells[cellIndex + nextCol - nextRow];
                            w  = c;  c =  e;  e  = cells[cellIndex + nextCol          ];
                            sw = s;  s = se;  se = cells[cellIndex + nextCol + nextRow];

[...]

                            var sum8 =
                                    (nw & 1) + (n & 1) + (ne & 1) +
                                    (w  & 1) +           (e  & 1) +
                                    (sw & 1) + (s & 1) + (se & 1);

[...]

                                var tableIndex =
                                    (((c         >> plane) & 0x03) <<  0) |
                                    (((e         >> plane) & 0x03) <<  2) |
                                    (((w         >> plane) & 0x03) <<  4) |
                                    (((s         >> plane) & 0x03) <<  6) |
                                    (((n         >> plane) & 0x03) <<  8) |
                                    (((cellX             ) & 0x01) << 10) |
                                    (((cellY             ) & 0x01) << 11) |
                                    (((phaseTime         ) & 0x01) << 12) |
                                    (((plane     >> 1    ) & 0x03) << 13);

[...]

When the sub-expressions won't fit on one line, you can still use indentation to help the reader understand the spatial patterns:

                            error +=
                                (nw * kernelBytes[4 - kernelDown - kernelRight]) +
                                    (n  * kernelBytes[4 - kernelDown]) +
                                        (ne * kernelBytes[4 - kernelDown + kernelRight]) +
                                (w  * kernelBytes[4 - kernelRight]) +
                                    (c  * kernelBytes[4]) +
                                        (e  * kernelBytes[4 + kernelRight]) +
                                (sw * kernelBytes[4 + kernelDown - kernelRight]) +
                                    (s  * kernelBytes[4 + kernelDown]) +
                                        (se * kernelBytes[4 + kernelDown + kernelRight]) +
                                frob;

But that would probably be more readable if I used intermediate variables for the kernelBytes[...] expressions, gave them descriptive names matching their corresponding directions, and lined up all their calculations so you could see the similarities and differences, because right now all the kernleBytes[4 +/- kernelDown +/- kernelRight] expressions are jumbled around horizontally, when they could be lined up nicely by themselves if they weren't interleaved with the n/s/e/w/ne/nw/sw/se multiplications. (I'll leave that improvement as an exercise for the reader. ;)

The point is you want to vertically line up as many of the repeated letters and symbols in nice neat columns as possible, so that your eye can easily see which are the same, and the ones that aren't stand out obviously so you can ignore all the same things and focus on the differences. (Which are typically variations like +/-, sin/cos, x/y/z, -1/0/1, nw/n/ne/w/c/e/sw/s/se, and longer series of names and numbers. Whatever changes should stick out visually, and be lined up and grouped together so your eyes can scan over them!)

That not only helps other people understand your code, but it also helps you be sure that you wrote the right thing, what you actually meant, instead of making an easy to miss typo.

The difficulty of reading you own code, which is HARD even for professional programmers, is that you usually see what you MEANT to write, not what you actually wrote. That's why it's so important to get other people to read your code, and to read other people's code, like pair programming and code reviews, or even explaining it to a duck.

https://en.wikipedia.org/wiki/Rubber_duck_debugging

It takes a lot more effort and concentration to slow down and look at what's actually there, instead of what you want to be there. (That's true for many aspects of life...)

it infuriated me that Google's style guides forbid this kind of formatting. Anytime someone else tried to get them to allow it it was shouted down as ASCII art

If I'm honest, I like that kind of formatting for small 2-3 line, 2-3 part expressions. But I am not a big fan for it in much bigger parts (like that huge index calculation above). I always see them and think that I'd rather see a loop over an array of values, or split into many smaller expressions.

Nice of you Steve.

Liam - whatever Steve suggested are very good pointers but please remember you are getting accolades here because you cared to publish your code. Don't hesitate to publish your initial version without any hesitation in future also.

Kudos for taking the time for doing this! It's places like these where the HN community really shines.

Thanks a lot!

Thanks! I really enjoyed making it. HN has been such a huge part of my life over that last ~10 years that it's truly the least I could do.

Steve, it is outstandingly awesome that you did this and also how you did this! Thanks.

Thank you, much appreciated!

Incredible code review!

I wish this inspires clever people to produce huge amounts of parody-videos "refactoring" the same piece of code. :)

I'm a long time coder and still liked watching your video just for watching someone else work in emacs. How do you get emacs to highlight all occurrences of map? Was it a keybinding for "highlight-phrase"?

Thanks, glad you enjoyed it! And like another person already responded, this is vim with a split pane where the right pane is a `:terminal` (a relatively new feature of vim... though prior to that I would have just used tmux).

Highlighting searches is enabled with `:set hlsearch`, and you can search for whatever word is under the cursor with ``. So I just type and it highlights all of the matching words. Nothing too magical!

This is vim.

I think I just died.

WHAT YEAR IS IT

Plausible deniability: It could have been Emacs pretending to be vim.

I'm subscribing to you, just incase you do more video's on code reviews.