02:22 - Elijah Manor Introduction

04:49 - What is a “Code Smell”?

10:21 - Copy/Paste Code Error

13:11 - Using ES6 to Eliminate Code Smells

15:48 - Refactoring Case Statements

21:29 - Juniors and Code Smells

27:29 - Isomorphic Code

31:12 - Framework Code Smells

33:47 - Identifying New Code Smells

36:33 - When Code Smells are OK

39:10 - Why use parameters?
On YouTube
188 JSJ JavaScript Code Smells with Elijah Manor

Picks

Terms And Conditions May Apply (AJ)
Nodevember (Aimee)
Developer Tea (Aimee)
Jake Shimabukuro (Joe)
Screeps (Joe)
react-styleguide-generator (Elijah)
react-styleguidist (Elijah)
The Phantom Menace - What it Should Have Been (AJ)
Attack of the Clones - What it Should Have Been (AJ)

Special Guest: Elijah Manor.

 

Do you want to host, AJ? Or I'll be the host then?

You go ahead and take it, Joe.

Why, thank you.

Hello everybody and welcome to episode 188 of the JavaScript Jabber Show. Today on our panel we have AJ O'Neal.

Yo, yo, yo, coming at you live from the place that always rains.

Where's that?

I'm still in Oregon. I'll be back in Utah next week.

We also have Dave Smith.

Hey, yo.

Jamison Dance.

Hello.

And last but certainly not least, Aimee Knight.

Hello.

And I'm Joe Eames. I'll be your host for today. Chuck is not with us today. Hopefully, he'll be with us next week. And we have as our very special guest, one of my absolute favorite people, Elijah Manor.

Thank you from Tennessee.

Elijah, do you want to take a quick minute. I know that we're going to be talking today about code smells. But do you want to take a quick minute first and just give us a bit of your background and bio?

Sure. Yeah, I've been developing for quite a while. I started doing Microsoft stuff back I first developed. And then more and more I really liked to be very focused in what I do, so I focused on the frontend of the .NET stack. And then when MVC came out, I started doing more JavaScript, more heavy JavaScript. Actually got heavy into jQuery because that was the thing and met a lot of the jQuery core contributors and actually briefly co-hosted the jQuery podcast for a while. Stepped back and then became just a full-on frontend developer, technology agnostic, whatever the backend, didn't matter. And I worked for appendTo quite a while with a lot of the jQuery core members, which was pretty cool.

And I worked at home, which is great. Did some Pluralsight courses. I know many of you have done some courses, and that was great. And I worked for Dave Ramsey. I don't know if you've heard of him, but he does a lot of financial coaching and things like that, and that was great. But I really loved working remote and at home, so I just recently joined LeanKit which allows me to work remotely. And their offices are actually really close where I live, if I need to come in. But I like working at home. But yeah, I love JavaScript and frontend development. Particularly these days I do React and ES 6 or 7, whatever suits my fancy, with Babel. A lot of Sass. Really, I speak at quite a few conferences. And typically the things I speak about are things that I've had pain in, in the past, so things I've learned. I'm like, “Wow. That was awful that I used to do that.” I'll try to learn something new and then I want to share that with people. And so, one of my recent talks was JavaScript code smells and just things that, code that works in the browser or works in Node or wherever. It's just when you look at it, it's like, “Hmm, this doesn't feel quite right.” And more often than not, it's either maybe I haven't learned enough tooling yet to solve problems in different ways. And so, it's a nice exercise in thinking outside the box and figuring out maybe design patterns that might solve certain smells, and use tooling where it's possible, like linters. And if linters don't exist, then make your own rules. Just try to support and reduce the smelliness.

Awesome.

So, did we already define what a code smell is?

Sort of, I tried to right there. We could actually define it. [Laughter]

What is a code smell and how do you smell it? [Chuckles]

It's kind of subjective. It's just… and typically if you're a beginner, like a complete newbie, you might think all your code is just fine. It's just as you start learning more things, you realize you just need to abstract methods and refactor and all these things. You start recognizing patterns that you're like, “Wow. This feels a little repetitive.” And typically you're like, it's something that feels repetitive but you're not sure how to pull it out yet. And typically that shows a growth in your growth curve in seniority, which is a good thing. So, it's actually a really good thing to sit down and think, “This kind of smells but I don't know how to fix it yet.”

And so for example, like when I gave this talk, a friend of mine at work, he was actually doing a code review of someone else's code. And he's like, he pulled me aside and was like, “Elijah, I think this smells but I don't know what to do with it.” And so, it was a great opportunity. And I actually added a whole new point to my talk about that whole experience. And it was essentially, he was passing data into a method and it was returning the output of the new data. And he was passing that data into another method and then saving it off and passing it into another method, saving it off. And it was obviously very repetitive. And so, it gave us the opportunity to be like, “Oh, well let's start trying it different ways.”

And one, we put them in an array and we did a 'for each' but we were like, “Eh,” we didn't quite like that because the 'for each' had to know a little too much about the context of what data it's manipulating. So then, we used a reduce and passed in the context. So internally, it was building up the end result. We thought maybe that was a little too clever or maybe not everyone would understand. And then we eventually went to Lodash and found the 'underscore dot flow' method that you could actually pass in an array of things and pass in the data and it'll actually make sure it flows through all of them. And so, it's really just an indication of, “Hey, this doesn't seem quite right because I'm learning more and I'm realizing it's repetitive. So, how can I make this better?”

Now obviously, some of the smells I have in the talk aren't about repetition. It's just about, just certain things that I think are smells. We can get into those, because some of them might be somewhat controversial.

Is there like an accepted repository of code smells? Is it like design patterns where there's some general shared knowledge of 'these things are code smells'? Or is code smell… are they more subjective?

Now, there is a list.

Let me answer that. There is a repository for [inaudible].

Oh, gosh. Be careful what you say. [Laughs] [Chuckles]

Now Martin Fowler…

That could go really bad.

I think originally defined what code smells were. And I'll have to try to find the link for all of them. But many of them, they're not specific to any particular language. Pretty much like you have too many parameters that you're passing in. And many of those code smells are about highly OOP, like object-oriented programming, how you would solve some of those. And they don't necessarily map one-to-one into a dynamic language or a functional language. And so, what I try to do is take some of the common ones that Martin Fowler identified, but then pick some that are a little more specific to what we do in the frontend world.

So, if I could break in here. I just looked up Martin Fowler's official definition, which I really like. And I think this is going to be useful to talk about. He says, “A code smell is a surface indication that usually corresponds to a deeper problem in the system.” And he says, “By definition, it's something that's quick to spot or sniffable, as I've recently put it. A long method is a good example of this. Just looking at the code, my nose twitches if I see more than a dozen lines of Java. The second is the smells don't always indicate a problem. Some long methods are just fine, for example.” And then he goes on to talk about it. The best smells are easy to spot, most of the time leads you to interesting problems.

Yeah, that's a great definition. And some of those real easy ones, linters like JSHint or ESLint can quickly pick up. Like you could do max statements that both of those support. And you could pick an arbitrary number like, I don't know, 15. So, if it looks through all your code and you have a function or a method that's above 15, then sure enough, ESLint or JSHint will give you an error or a warning, which is great. Some other really common, easy ones that currently are supported in linters are like max depth. So, you don't want a for loop with an if and then a while and another for. You don't want really nested code, because it's harder to get your brain around all that complexity.

Speaking of complexity, there's a max complexity which is used, the cyclomatic complexity score. It's been around in many languages, but JSHint and ESLint actually, that's one of the rules now, which is great. So, you could pick an arbitrary number of complexity, which really just measures all the branch logic within your code. The smaller the number, the easier it is to unit test, easier to just get your head around it. One of the cool ones that ESLint provides that none of the other ones provide yet is a max nested callbacks. So, it prevents the cascading of doom that you get when you have a callback inside of a callback inside of a callback. So, having that a really shallow number is a lot healthier code and it just doesn't smell as bad. So, those are some really easy ones. And that I think all of us would be able to sniff those, like really long methods.

But from there, I think it gets a little more complicated in figuring out what is smelly. And that's really what the rest of the talk was trying to identify, things that weren't as easy to spot.

Do you have an example? You already talked about that first one where you were doing a bunch of…

Yeah.

JAMISON: computation on the same data.

Yeah. So, another easy one to spot that there is a tool thankfully, is just a copy paste code error. Because typically what happens in… you know, of course none of us would do it but the people that we work with right? We'd find some code that we wrote and it's like, “Oh, that solves my problem. I'll copy that and paste it over here and tweak it slightly.” And if that keeps happening over and over, obviously it's a smell. But it becomes problematic in maintainability because if there was a bug in one of those or you want to change it, you have to find all the instances.

So, there are two tools. Jsinspect, they're both Node modules, and jscpd, JavaScript Copy/Paste Detector. Both of those will run it over your whole project and they'll try to detect within a certain, you could actually change how many tokens it actually is trying to determine if they're the same or not. So, you can make it more specific or more general. But both of those tools, and I'll provide the links, are really helpful to run against your project just to find those. And those would be possibly good areas to do refactoring. Hopefully you have unit tests to make sure that you're not breaking more things that you're fixing.

So, I ran those against our particular codebase. And I was actually a little concerned when I did it, [chuckles] because I didn't know what the result would be. But for both of them, pretty much they found the same thing. We have transformers or adapters, like after we do an Ajax call into our server and get the data back, we manipulate it a little bit to conform to a certain object structure that the rest of our app understands. And they found some duplications in those transformers. And I was kind of okay with that because they look similar but they're very different. And so, but yeah, for your app you could run it with your Gulp system or Grunt or if you use [inaudible] scripts, have that part of your build process to find those.

But that's an obvious one. You're asking what's a harder one to detect?

Oh, I have one question about these before you move on.

Oh, sure, go ahead.

How do they work with, if you're using ES6 or ES7 or Babel-ified code I guess? Do they understand that or do you have to compile your code first and then run it on the compiled code?

Yeah, so the second one, the jscpd does support a lot of different languages like JavaScript, TypeScript, C#, Ruby, CSS, SCSS, and HTML. And I did run it against our project which we were using Babel, ES6 and 7. It didn't complain. And I didn't run it against the compiled result. So, I guess it does…

Okay.

As far as I know, it does support it. I'll have to make sure for certain. But it does support many more languages, which is great because oftentimes we have the same problem in our Sass, in our CSS, a bunch of duplicated things. So, it might be helpful there. The cool thing about it, you could tell it which languages you want to support and which file types, and how many tokens, how specific or general it is. Good question.

In part of your talk, you talked about different ways that you could use ES6 to eliminate some code smells. I feel like the community hasn't fully embraced ES6. So, that might be a good way to talk to people to encourage them to take a look at it.

Yeah, I tried to learn it myself a little bit in how I refactored some of the code. Because typically in the talk, I'd show some smelly code, typically code that I wrote just because it's easier to… I'd rather poke fun at myself or things that I've done. Because I pretty much, everyone in our journey of learning has made these mistakes. And they're really, it's not mistakes. They work. It's just you want your code to smell better every time you look at it. And so, some of my refactors that I would show, and first I'd encourage people to have unit tests. I was like, “Before you refactor, you don't want to break your code.” Oftentimes I would refactor using ES6. And I typically just would brush and say, “You don't have to do it this way. This is just an exercise.” But it did make many things a lot cleaner.

An example of one of those, I called it the this abyss smell. And so many people, developers from beginner, mid to senior get tripped up with this and the context and all that stuff. And so, I would show a constructor function using this and had a 'for each'. And so, they saved this off to that, so they could use that inside the for loop. And I mentioned, “Hey, it's okay that you're saving off this as that. But more often than not if reveals that as a developer maybe you haven't learned other techniques yet.” And so, that's where we talk about bind, talked about the fat arrow in ES6. And then even some cool things that not everyone realizes, like the 'for each' method off an array actually takes two parameters. The first one's a function and the second parameter is actually the context you want to be used inside the first function. So, you don't even have to say bind if you didn't want to.

So, just going through some of those exercises is kind of fun. And there are actually some really cool things in ESLint for that. There's actually a rule called 'consistent this'. So, if you want to save off this as that, you could actually tell ESLint, if I ever do that, it has to be called that or self. But one of my favorite ones is called 'no extra bind'. It's a rule in ESLint. And that one goes even further where if you ever bind a function to change the context but within that function when it eventually gets invoked, if you never use the this implicit parameter, it will gripe at you. It's like, “Why did you even bind this? Because you don't even need to because you never actually used the this implicit parameter.” So, ESLint's great because it's actually an AST parser and it actually picks apart all your code into branches and figures out the execution life cycle. So, it can do some really intelligent things like that.

I also really like the section where you talked about refactoring the case statement. Would you want to talk about that?

Yeah. So, that one's possibly a little controversial.

A little hard to explain too, on podcast. But…

Yeah, so particularly I suggested that just having a switch statement at all [chuckles] is a smell, for a couple of reasons. One, the Open/Closed Principle. One of the Uncle Bob SOLID principles suggest that when you have a piece of software, it should be closed for modification but open for extension, which when I first heard that I'm like, “Those sound like great words but I have no idea what that means.” But the more and more I learned about it, it's like, “Oh, well when I write some code, I probably don't want to touch it again.” I want to make it so it's just extensible because if I add a new feature to it, if I actually go in and tweak existing code, the probability of it actually breaking is a lot higher. So, it would be great if I could just keep old code as it is and add to it later. And so, with a switch statement, you're essentially going back and changing all the cases. And so, you could possibly break the rest of the code.

So, one of the design patterns of the Gang of Four is called the Strategy Pattern. And typically if you see a switch statement you can most likely convert it to the Strategy Pattern. And so, what I did in this particular example is I used if-e's just because I didn't want to introduce the idea of native modules or CommonJS or all that. So, I used an if-e and each if-e defined a particular type. And so, the switch statement was getting the area of some shape, like a triangle or a square or a circle. And so, each if-e just defined a class of just a triangle, just a square, and had the prototype stuff. And then what I would do is if I had a new type, I would just register that type onto some common object that has all the types that I support. And that way, if I wanted to add a new type later, I just create a new file called circle.js. It registers itself to the shapes object and just works that way, instead of actually going in and modifying existing code.

So, I created a rule. It's a really silly rule. And ESLint pretty much just doesn't let you use the switch statement. [Chuckles] Which is kind of overkill. So, I created another rule, 'simple case statements only' or something like that. I'm typically okay with switch statements if it only has in each case statement, there's only one line. But what typically happens is, let's say I have a switch statement and each case has one line in it, of course with the break, what typically happens is you'll start adding more and more code into those case statements as you realize maybe it doesn't fit your needs. And that's where it gets out of hand.

And so, typically with any of these rules, JSHint and ESLint, if you have exceptions like you're pulling in code that you didn't write and you're not ready yet to refactor that switch statement, you could disable the rule for the whole file. Or with both of those linters you can actually put a comment saying, “I want to start ignoring linting now,” and then have another comment, “I want to re-enable linting.” So that way, you could still have old code that you identify as smelly but you're like, “It's okay for now.” And so, yeah it's a little hard to describe that one online. But there are slides that you can reference to later. But essentially that's the idea, trying not to modify existing code that already works.

So, one thing, since you bring up if's and switches and cases and stuff, that I notice… this is something that just makes my skin crawl, is people put an if and then inside of that if they check for the success condition, and then have a huge block of code that ought to be a function, and then else error condition, which is usually like return or exit or something. And they nest these things, like six things deep. And what you should do is do, if error condition, return error code, or reject promise, or whatever. And then you never indent. And if something's getting big it's like, if this operator is used, then replace those 50 lines with a call to a function, else the other operator is used. Replace those 50 lines with a call to a function.

Yeah. Typically like you mentioned when you see extra nesting or really big methods, both of those are just screaming like, “Pull me out.” [Chuckles] “Refactor me.” It makes your tests a lot easier, or unit tests a lot easier to write. It makes the code a lot easier to grok or understand. Yeah, there are many of these smells. Again, some of them are highly defined, like the Martin Fowler list. And some of them I just tried to… like that one, I don't think there's a particular name for that smell. That would be a good one to maybe… I imagine we could make a list really long based on our experiences.

Isn't it cyclomatic complexity? Or…

Well, that encompasses lots of things.

Or nesting.

Yeah, there is a nesting one. But if you had a pretty shallow nesting, it still might pass. But what you were saying, like the initial return punt, it wouldn't necessarily catch that. I mean, what you're saying, that's a good thing. But…

Well yeah, there's, what do they call it. I'm trying to remember. In my Computer Science class we called it something. I think we call it the same thing in regular programming. But you want to find your base condition. Base condition checking.

Yeah.

You want to find your base condition, which is the condition that exits you from your loop or your program cycle, or your function.

Yeah, definitely. But yeah, you could possibly, it might have to be pretty smart, but you can make a custom ESLint rule to see if there are violations of that. That might be kind of interesting.

No, I think I'm just going to look over people's shoulders and nag at them.

[Laughs] And that, too. [Laughter]

Yeah, you'd be surprised.

That would [inaudible] ESLint.

[Laughs] Jamison knows all about that.

[Laughs] That's true.

That actually brings me to another question. So, I had two questions first off, because I like to be the voice of people who are coming in, newer to programming. So, the first question was, if you ever see juniors who make a consistent code smell. And then also, what if you're a newer programmer on a team and you see someone who is higher up than you writing these code smells. How do you approach that? [Chuckles]

Yeah, the second one's a little bit harder. But the first one, pretty much all the basic rules that I mentioned that all the linters do exist already, like max statements, max depth, complexity. Those are the ones you're probably going to run into most frequently, or the copy/paste. I see all that a lot. So, just visually, it's like, “Wow. These look so similar.” Because we're just trained over time to not do that, the longer we've been in this particular career. And so, you're just not sensitized as much when you're a newer developer. You're like, “Oh, it just works if I just copy this.”

But the reverse question, well and just this, the context of this, it throws everyone for a loop. I probably get that question most when I'm helping junior to mid-level developers trying to get past that. Because the thing is, they could get most of it to work. It just might smell. But if you get the context of this wrong, things just break. So, [laughs]…

Yeah.

That's typically where like, “Hey Elijah, can you help me?” And then while I'm there, I try to look around and find for other things that we could talk about. And I try to make most of my conversations just a back and forth learning experience and that's kind of fun.

But to answer your second question, the best thing that I've seen with that is just when you do, like for every person on the team at Dave Ramsey and for every team at LeanKit, any time you do work you make a pull request and at least one other person has to review it. And I think that's the best place for cross learning, for actually juniors to look at seniors' code and not only learn from it but to ask questions like, “Oh, why did you do that?” And sometimes it's like, “Oh, I did that for this reason.” And they're like, “Oh, great.” But sometimes like, “Oh, you just, you noticed that I didn't use this correctly. Or I could refactor that a lot easier.” And so, it provided them opportunity to actually… so as a senior, we might already know that, but we're like, maybe we're just lazy or something like that. And maybe we didn't. So, it gives an opportunity for the junior people to breathe life into the seniors and also for them to see how we do things.

Yeah.

And which could help them, too. And even seniors reviewing, like peers reviewing peers is great, too, because sometimes a system is so large, you just don't know all the stores or actions that exist or helper methods. Or even Sass mixins like, “Oh, did you know this mixin existed?” “No, I didn't.” So, it's a great time for cross learning, for mentoring, and also helping juniors actually help things get better. So, I would think that would probably be best. That's probably the least intimidating. It's still somewhat intimidating if you're a junior adding a comment on a senior. But most of the developers that I've worked with have a pretty healthy ego. And I think the way you could kind of…

What a nice way to put it. [Laughter]

Healthy ego.

And I think the thing that I've seen that has worked best for seniors to lower their ego is to have a [heart] of a teacher and a mentor, because it constantly reminds me and whoever else where we were when we were first learning. Because once you forget that, where you were, then that's where I think the trolling and the like, “I'm better than everyone,” sinks in. Maybe I'm wrong, but I think it helps us be grounded if we have that kind of mentor relationship with someone else. So, I think it's important.

I would approach that by saying, “This looks really stupid of me, so I mustn't understand. Can you tell me why this isn't stupid?”

Yeah, and might totally…

That's totally the best way to do it.

[Laughs] Definitely, it's how you approach it. You don't want to be yelling at people.

This looks really dumb. Like, this is like something an idiot would do. Is this right? [Laughter]

Might not be the best way to approach that. But… [laughs].

That's how Aimee handles her…

[Chuckles] No.

Sometimes I don't know how else to say it.

Code reviews are great, though. I've been really fortunate. I work in an environment where we do that. But I know not everyone, a lot of people I know who are juniors don't necessarily have that environment. So, I was hoping to ask the question in case there were others out there who were wondering.

Yeah. I would think that if you work in a job that doesn't have pull requests or peer reviews, maybe try to suggest it as something that you could incorporate.

Yeah.

Because it's one of the best things that I've seen. Because at jobs where I didn't have it, I didn't feel pushed as much, or made accountable for things as much. But as soon as that started being a part of my job, it was like, “Oh yeah, someone's going to be looking at this.” Because sometimes you just get lazy or you forget something. And as soon as someone points it out, you're like, “Oh yeah, I forgot,” or, “Oh yeah, that's a great idea.” Yeah, I think it's a very healthy thing. Do you all mostly do that at your positions?

We do. And I know for me, it is the most satisfying thing in the world when I'm reviewing someone higher up than me and I find things and I point them out. [Laughs] [Chuckles]

Obviously in a nice way. But it's like sometimes…

Yeah.

Yeah. And yeah, it's showing that you're understanding it. Like for them, like, “Oh wow. She's getting it. She understands. She's tracking.” Those are all very important things.

I just claim that any mistakes people find in my code I left there intentionally to test them and they passed.

[Laughs] Good.

You know what else too, my mentor that I've been working with, he has this awesome insight that he said. If you have to intentionally leave something smelly for some horrible reason, you should put the reason in a comment next to the code with your initials and a date.

Yes. Doing binary or here because a bug in Chrome v8 makes it faster in Node. [Chuckles]

Which happens.

Yeah. And I think Lodash does a pretty good job of, they add comments for code that's really wonky that they're doing workarounds for browsers and stuff like that. So, I think that's a good use of that.

So, I want to bring up something that I've noticed recently. Now, I'm a proponent of isomorphic code, to an extent. I'm a proponent of the idea that you can write, this is the key difference, that you can write a module in such a way that it can be used both in the browser and in Node. So, you're taking the algorithm that's the important part, you're abstracting that out and assuming that you're using a typed array or an array or whatever, and that that's how you're going to manage bytes if you need to manage bytes.

Right.

Or you pass in some function that, like a dependency injection style, you're passing in the function that does the weird conversion wonky thing, and that's your module. But I've just this week come across a couple of cases where, well in this past month, where people have browser hacks in Node code because they're copying and pasting browser hacks. Or people are trying to load stuff that really just deserves, to me it seems like it deserves a second module. In Node it makes sense to do it this way because you need to do a thousand operations per second versus in the browser you ought to do it this way because you just need it to take up fewer bytes. Like that kind of thing. That's something I've been noticing recently. And I don't know what you call that. But when I look at code and I see that this is obviously for the browser, this is obviously for Node, but you've got 50% of your code is trying to get it to work across Browserify and Node and Require and yeah.

Yeah, it just knows too much about… it shouldn't have to know all those things. So yeah, I don't have a good name either. Another thing that's very similar to what you're saying but slightly different is, and it seems to be a reoccurring thing. So, like when jQuery was really big, everything you found was a jQuery plugin, even though it didn't have to be a jQuery plugin. Then if you switched to Dojo, then you're like, “Oh, well I got to punt that and I have to find another one.” Or now, the hot thing is like, “Oh, here's a React component.” But good luck finding the underlying just library.

And so, I think a better way and even back in the day it was a better way, have a reasonable, common, vanilla library, JavaScript library. And then make adapters on top of it for jQuery or for React or for Angular, for Angular 1 versus Angular 2. And that was an exercise I also did at Ramsey Solutions with another developer, Damon Bower. And he inherited a jQuery plugin and I was like, “Hey, instead of just rewriting that, let's actually abstract the common piece of code and use Babel or whatever you want. But actually have a really clean API and then we'll make a jQuery adapter on top of it because that's what many of the websites use, jQuery for their sites.” And that way we can make our… it's really easy to unit test, the low-level vanilla JavaScript library. And then we can, once we've made the adapter around it with jQuery we could write unit tests to make sure it's passing through.

Yeah.

And so, I really like that approach a lot better, because then the React project we were doing for Every Dollar, the online budgeting tool I worked on, if we ever needed that then we could just make a thin React component that talks to that library instead of having to rewrite it from scratch. And so, it's similar to what you're talking about because the problem that you were mentioning knew too much about the environment. But I see that theme over and over. And I'm sure you all see it too. Like, “Oh, here's a Dojo-specific thing, or Ember-only component.”

Well, and Node. Node suffers from that a lot.

Yeah.

Because people write really good code. And they're like, “Oh, this is only going to be used in Node.”

Right. Yeah, yeah.

And so, they really heavily rely on certain Node-isms that they could abstract a couple of those functions out, and then you just, you'd be able to use it in the browser and it'd be great.

Yeah, definitely.

So, this brought a related point to mind. It seems like part of the speed up that comes from using a framework for while is noticing that there are, it seems like there are generic JavaScript code smells but it feels like there are also framework code smells. Like if you're a…

Definitely.

heavy Ember developer, there's probably some Ember code smells or Angular code smells or React code smells. Do you see those on the same level? Or do you think, everyone just learn the JavaScript ones and the framework-specific ones will come over time? Or do you see a relationship between those two?

Well…

Yeah… Oh, you go ahead.

I just want to get a clarification. What do you mean, like a React code smell or an Angular code smell? You mean doing regular JavaScript in the wrong way?

Like I would say that that code smell is using state too much, using 'set state' everywhere instead of using props more. It's kind of vague, but I don't know. They're just general guidelines of things that you learn specific to the framework that seem separate from purely JavaScript-specific things.

Would that be like a style guide?

Yeah, maybe. But I mean then would regular code smells be part of a style guide, too? I guess we kind of talked about JSLint or ESLint, so that's kind of a style guide then, too.

The nice thing about ESLint is many people from each of those frameworks have created a package of rules to try to quantify what smells might be in that language, or that framework. So for example, ESLint-plugin-react, we pull that in as well. And there are many cool things like you could actually force that you use prop types, if that's something that you want to do. You could force that you add keys to things that need keys so React could do its magic. You could actually prefer ES6 classes when you're doing React components, if that's the type of thing you like. And there's tons of other, probably like 30 specific rules that are specific to that framework, best practices that your team feels is necessary.

And the cool thing about it, there's one for Angular, there's one for Ember, and one for Backbone. And so, each of those particular, like, “Hey, you should make services,” or whatever the thing is in that particular framework. But the cool thing is, if there's more that you find appropriate for your stuff, you could either contribute to those repos or make your own set. Yeah, that's a great question. And we do use the React ones against our projects because they are quite nice.

It seems like in some ways, code smell is a fancy word for experience, because there's not this defined list of 20 code smells and nothing else is a code smell. It's just like, you learn enough to see when something can go bad later on. How do you get better at identifying new code smells?

That's a great question, because some of the ones… so, the talk that Aimee saw was really short. But normally like an hour or just a little bit longer talk. So, there are things I talked about in the longer version that are kind of just experience. Like one I called the incessant interaction smell. And it's a little less JavaScript but it's more just from a UI standpoint. Like think of an autocomplete box for example. When you're typing in, you don't want each keystroke to actually make a call to the backend server, because your backend friends would not like you. [Chuckles] And you could DoS yourself, which would not be good.

And so, the first thing to think about, “Oh, I should throttle this, or at least have a min number of characters.” But even throttling doesn't quite do what you probably would want it to do, because what throttle does is typically… and Lodash or Underscore have throttle, or Ben Alman has a version he wrote a while back. But what throttle does, you give it some interval like, “Hey, every 500 milliseconds, or I only want to make one call every 500 milliseconds, at least.” Or at the most, I guess. But what could happen, someone could be typing and typing and typing, and just really quickly. And it could be making several calls. But from a UX standpoint, that's still probably too much, because they haven't really even paused to take a breath or even think about what they just typed. Probably you want to autocomplete when they've stopped for half a second or 250 milliseconds. And so, I still call that a smell.

There's something called debounce, and for a long time I never knew what debounce meant. But again, Lodash and Underscore have it. And it's the same API as throttle, but what it means is if I say debounce, wrapping my function with a number, that means you have to wait until that event at least stops for that long. So, I can be typing, typing, typing, backspace, backspace, backspace, type, type, type, and then pause for half a second. And that's when one call goes out to the server. And so, I call that a smell. It's maybe more of a UX smell, but it's really how you solve it with JavaScript so I kind of added it in there.

But that could be really handy for autosave. So for example, I worked on a project for a company where it was like a WYSIWYG type email editor. And every maybe 15 seconds of non-activity of certain events, we would just autosave a version that they were working on. And so, debounce was really handy for that. So, that's a particular one that was more from experience, like you just mentioned. It wasn't like a hard set rule like, “You should always do it this way.” But yeah, just over time when I learn something and want to tuck it away and realize maybe not everyone else knows that, then those are the things I like to share. I'm like, “Hey, I made a mistake or I did this and it was painful. Here's a better way.”

So, one of the things that I'd like to talk about, we briefly touched on this when we talked about the definition of code smells, is code smells where it turns out you don't want to make a change, you don't want to fix. It's actually not a problem. How do you go about identifying those and deciding, “Oh, even though this looks wrong, it's actually okay, I'm going to leave it the way it is”?

That's a good question, too. And I guess part of the answer depends on if your rules are triggered against it. And if it is, then that's where I just put comments. I just disable that rule for a particular block. Early on before I was using ESLint, JSHint would complain about a lot of the ES6 and ES7 stuff we were using. So, we regularly had to tell it to ignore certain chunks of code. And I'm trying to remember, I think it was when we were doing rest and spread and destructuring some of those. We got confused. But since it's gotten a lot smarter. And so, in that case we actually didn't think it was smelly. It was just the tools we were working against.

If it was really something that we felt was smelly but our tools didn't catch it, probably wouldn't worry about it too much. I don't even think it would deserve a comment. If it got through all the pull request reviews and we felt good about it, I wouldn't… I typically don't like to put extra comments for things I don't feel are extremely necessary. I don't know. I guess it would depend partially on what that smell is. Do you have an example or it's just more of a high level [discussion]?

Well [sighs] like say that you have a method that takes in eight parameters, which is a lot of parameters.

Ah, gotcha.

But you look at it and you decide, you know, I could break this down but it just doesn't feel right to break it down. This just makes more sense to leave it. Or you know, maybe you've got a method that has an extreme number of lines of code in it and you just feel like, it's okay the way it is.

So, in those cases, we definitely, I would have rules for all those. And then just, I would put a comment to ignore it. And then that would be an indication for whoever either, one it's okay, or two someone else in the future could actually go in and refactor it if they wanted to.

Or maybe, because you'd have to disable it either way, maybe disable and add a comment like, “This is okay. You should not refactor this,” because maybe there's a hard dependency. Maybe you're integrating with another library that has to have specific parameters or something like that, or to receiving those parameters, and you don't have control over that. That would be an appropriate place to say, “This is like this for a reason. Don't refactor it.” [Chuckles] This signature is important. Above that, I'm not really sure. Or maybe you could write, you could use JSDoc or something like that, list out all the parameters and say it's actually in good reason, like this maps to another library or something like that.

Right.

So, here's a question I have related to that. Why use parameters at all? Why not just always pass in a single object? This is just something I've thought about recently.

Oh, definitely. And that's usually what I say when I give a talk, is the first smell I do is just some really nasty code I wrote, I don't know, six years ago in a blog post. [Chuckles] And so, I was passing tons of parameters. I'm like, “Hey, if you could save… if it's over four or five parameters, maybe you should pass in an object instead.” But to the point we said earlier, maybe there's some integration that it has to have that signature for it to work. And you don't have control over how it's being called. But generally, I think objects are much nicer because they're a little more selfdescribing. Because once you get a really long signature, and you're trying to invoke that signature, it gets really hard to tell what parameter's what.

Well, and you can't pass it to a promise. There are other areas where it just…

Yeah, yeah. So overall, I would typically prefer an object.

Although if you're looking at like code that's very hot code, that object thing could come back to bite you because that's memory allocation that's got to be garbage collected, depending on how you're passing the object and whether you're mutating or you're copying rather than mutating.

Yeah, and some of that you just have to figure out as you run it and as your exercise and profile.

An interesting one I brought up, and I don't always agree with it because it could go both ways, is anonymous functions. So, passing a callback and not giving it a name. That could be problematic because if you ever do profiling like we just mentioned, maybe you do have code like you mentioned that has a leak or just very memory intensive, if you use lots of anonymous functions when you pass in callbacks, then when you do stack traces or profiling you'll see all these anonymous, anonymous, anonymous. And it's really hard to figure out which is the one that you really want to actually figure out how long it took and where the memory is being taken.

Mmhmm.

And so, by just naming… you could still pass in an anonymous function but then give it a name, which obviously is not anonymous anymore. But that way, when you do a profile or stack trace you can actually map it to a particular name, which is really nice. And naming a function like that, it allows you to de-reference it. So, if you've ever wired up subscribe on an event bus or an event listener, if you actually give it a name then inside of it you could remove the even listener or you can remove the subscribe. So, it's like a one-time only. So, if I click on something, it could only fire one time and never click again.

So, there's some good reasons, and code reuse obviously. If you name all your functions, you could reuse them easier. But one of the downsides is the fat arrow, which everyone likes. If you use that, it doesn't have a name. And I don't think you could, you can't name that as far as I know, can you?

Not that I know of.

Yeah. So, that would still be anonymous. But I mention because when people ask me about that it's like, I don't necessarily name all of mine. But if I know I want to de-reference or have a nice stack trace or a nice profiling, then I'll give it a name just because. And that's more wishy-washy. Like some of the ones I gave very subjective, but just things to be aware of that they could bite you down the road. And some of that's experience, like you all mentioned before. But…

Right. Cool, do we have any more questions before we wrap up? Is everybody pretty much…

Smelled out?

Smelled out? [Chuckles]

Alright. Well, let's move onto picks then.

Before we get to the picks I want to take some time to thank our silver sponsors.

AJ, how about you? Do you want to start us off?

Sure. There's a film that's done in cooperation with Mozilla and the Electronic Frontier Foundation, I believe are two of the sponsors, called 'Terms and Conditions May Apply'. And it's an interesting documentary detailing privacy and legal considerations about what happens when you click 'I agree'. It's a little bit scary, honestly. And one of the funny facts that they mention, or perhaps not funny but I thought it was hilarious, is that if you were to actually read every agreement that you click I agree for during the course of a year, it would be 180 hours or a full work month of reading user agreements.

Wow. That's a lot.

And I will pick also not spending 50% of your code trying to make it isomorphic by detecting which environment it's running in, because you will lose. And then there are problems and it sucks. And I don't want to deal with that. [Chuckles]

Cool. Aimee, how about you?

Okay. So, I had Nodevember before. I'll call it a pre-pick. I'm going to pick it again as a post-pick because I feel like I've been really spoiled. I've gone to some awesome conferences, gone to ngconf and Angular Connect. But Nodevember was great, too. It was just like a really, really, really genuine vibe. There were great talks. All around, a great weekend. So, if you have the opportunity to go next year and they have it again, you should totally go. And of course, Nashville is amazing.

And then my second pick, I listen to a ton of podcasts. And some of them I'll skip around episodes because I only have so much time in the day. [chuckles] But I found a new podcast that I actually have not found an episode yet that I don't like. And it's called Developer Tea. They're just 20-minute very short episodes. But they're all very, very practical. So, I would encourage people to check that out. And that is it for me.

I'm going to start off by picking a ukulele player. Apparently it's not a yu-ku-le-le. It's an ukulele. That's what they said when I went to Hawaii. But there's this ukulele artist named Jake Shimabukuro. I think he's Hawaiian Japanese. And he is amazing. I've absolutely enjoyed everything that I've listened to of his. So, if you're looking for some really awesome, very enjoyable music to listen to, I highly recommended what he's got. Check him out on Spotify. Very good.

And then for my second and final pick, I'm going to pick Screeps.com, which is a massively multiplayer online game where you have to code to play. You don't actually play the game. You code up in JavaScript how your guys will play. And that's how you play the game. And it's super awesome. And if you are learning JavaScript it's a great way to force yourself to learn JavaScript very interactive. And I've only played around with it for a little bit. But I've really liked everything that I've seen. It's well-documented. Very interesting. So, that's my second pick.

Elijah, how about you?

Yeah, so I have two. They're related. So, when I worked at Ramsey Solutions, the project we were working on, it was React. And we had a component library or like a living style guide. But then it was actually written in Handlebars and a totally different technology than the rest of our app. And so over time, it got out of date. And we started there and then we pulled over the markup into a React component. But from then on, we just developed the React component. And it just, the React, the style guide died over time, which kind of stung. And so, I was on the lookout for a way to kick that off again by actually using the real React components that we were building. Because that way, they wouldn't get out of sync. And so, there's this project called react-styleguide-generator. And what you do is you create a component and then you wrap the real component that is in your system. And then you give it some metadata and it will generate this nice style guide for you based off the real things that you're using.

And there's another one called react-styleguidist. I haven't played with that one yet, but when I tweeted the previous one, someone mentioned, “Hey, here's one that might be cool, too.” So, I

definitely am going to look in both of those for my current job at LeanKit because they're wanting to create a style guide as well and didn't have it yet. So, I'm going to be digging into those.

And I think it’s a smart thing to have, because at Ramsey Solutions on other projects I was on, we had style guides. And it was really helpful for the designers and the UX people to have a common language of what's available. And then when we got comps of things to change, we could go like, “Hey, that looks different than our style guide. Was that on purpose or can we actually use something that we have before?” And it was a nice conversation piece. And also was helpful for the developers to know what classes should be used when. So yeah, I think it’s great. And hopefully they'll be helpful to others as well.

So, can I add one more? I forgot to mention this but I meant to. I've watched a lot of Star Wars rewrites for episodes 1, 2, and 3, because they obviously need to be rewritten. And hopefully one day redone properly. But the best one that I've found so far is called 'The Phantom Menace – What It Should Have Been' and 'Attack of the Clones – What It Should Have Been'. I've got the YouTube links there.

Cool.

How long are they?

The first one is about 40 minutes and the next one's about an hour and a half. So, they're feature length but they're done with stills and backgrounds. And then there's supposed to be one that's coming out for 'Revenge of the Sith – What it Should Have Been'. But they're complete rewrites from the ground up. There's hardly any similarities other than there's an Anakin and there's an ObiWan and there's a Phantom Menace.

Wow. That sounds really cool.

That's awesome. [Inaudible] check that out. Well alright, well thanks everybody for listening in. We're happy to have you as our listeners. [Laughter]

Gosh, audience. That's [inaudible] I was looking for, audience. Oh my gosh. [Chuckles]

You listeners.

We do this of course for the audience, so thanks for listening in. And thanks to Elijah for coming on and being our guest this week [inaudible].

It's been an [inaudible].

Yeah.

Thank you.

And for us as well. And we will look forward to seeing all of the audience next week, although we won't actually see you. But we look forward to you hearing our luscious voices.

Yeah.

And guys, if you just go on GitHub and star me, it makes me feel so good. So… [Laughter]

[Inaudible] star.

Or just check out my Twitter and retweet me. You know, just…

If you do go on and star AJ, please pick something completely innocuous and ridiculous. A repository he's likely to delete within a week. [Chuckles]

Alright, well again, thanks everybody and we will see you all later.