Refactoring with Deleting Code

[00:00:00]
>> Francesca Sadikin: Okay, so we have consolidated relevant pieces of code for every single item, but before we go into conjured items, I think there's another opportunity to refactor. Because I can see that, if I were to add conjured items right now, I'm just gonna be adding to this if-else, if-else, if block.

[00:00:22]
And I don't really wanna do that. So, what I'm going to do instead, is I'm going to extract these handlers out. So, I think what I'm going to do, is that I'm going to create a new variable or constant called itemHandler. Inside of which is an object and I'm going to use the name of the item as the key, and it's going to be a function that takes in item and then does the logic on it.

[00:00:55]
So I'm gonna actually paste this logic in and now I'm going to call it, right here. So, itemHandler(itemName), and then it's a function, so I'm gonna give it item. So I'm gonna fix save, reference my test suite, tests are passing. Cool. Delete code. Yes.
>> Speaker 2: Is there any rule thumb or etiquette to deleting code or branches of code like that in a real environment?

[00:01:28]
>> Francesca Sadikin: Well, in a real environment, you also have Git, so there is already version control for you. But when you're doing really large refactorings, you do just wanna make sure you really understand the code because existing code has layers and layers of bug fixes, and it's not always obvious.

[00:01:48]
So, yeah, like writing tests to make sure you really understood it. Try to find an engineer who has worked with it before to take a look at what you have captured. Also, I think ideally when you're working in production code, you wouldn't introduce a huge refactor all at once to production like that.

[00:02:07]
You might be doing a small little change, let it go into production, see what happens, looks good, then bring in the next set of changes. Like incrementally, instead of doing a big giant refactor and then, boom, production. So again, we continue with this refactoring. So, we already started with extracting the Aged Brie's handler out into this new data structure.

[00:02:34]
Let's keep going with Sulfuras next. So, here is Sulfuras as the key. This time same deal and creating a function with item and inside of it the function does nothing. A little JavaScript convention here is to add a little underscore to let other developers know that this is intentionally not going to be used.

[00:03:00]
And so, I'm going to call that line right here, plop it in. It's still passing. So, I know I'm copying and pasting right now, but we're going to just continue, and then we're gonna clean that up afterwards. So next we have backstage passes.
>> Francesca Sadikin: Going to add it here.

[00:03:23]
Here's my new function. Gonna paste all of this logic out.
>> Francesca Sadikin: And then call this line instead.
>> Francesca Sadikin: Okay, that's working. I can do, remove this, and so now we have the default handler. So what I'm gonna do is actually just create a new key, and I'll just call this one default.

[00:03:52]
Here's item,
>> Francesca Sadikin: Move this logic over.
>> Francesca Sadikin: And now, instead of calling item.name, I just need to change this to default. Let me see. And that passes, okay. So when I look at the final version right now, I can see that here is the duplication, right? We're actually really just calling this line over and over again, if it's a specialty item.

[00:04:27]
Otherwise, it calls default. So, what I'm going to do is actually extract this out, because the only difference is actually the name of the item. So I'm gonna call this a new variable called handlerName. And inside of it, I need a way to determine whether something is a specialty item or not, right?

[00:04:53]
So, we could, one of the ways that someone can do this is. We can create another collection called specialtyItemName. It could be an array with strings like this.
>> Francesca Sadikin: You could do this, and then we can use this here.
>> Francesca Sadikin: I just check whether the item name is inside of it.

[00:05:25]
Otherwise,
>> Francesca Sadikin: And if it is, I use the item name, otherwise I use the string default and then I call it here. So, instead of item.name, I press it's handlerName, let's see. So that refactoring works, but I don't like this way I'm doing it because now I'm copy pasting strings everywhere, like if add it here now I also have to remember to add it here.

[00:05:59]
It's very easy to make a mistake. So instead, another way that I'm going to do it is actually, I think I'm going to just extract the keys out of this object. That's another way to do it. So I'm going to just change that here. So, I could also use object that keys of item handler and then check it.

[00:06:27]
But I'm going this, what I don't like about this is now it's taking the keys, turning it into an array. And then now it's like a linear search to see if that item is going to be in this array, and then, over time, with more items, this is going to get expensive.

[00:06:47]
So, what I'm gonna do instead is use another method called. I want to say it's hasOwn, and it takes the actual object inside of it. And then the item name, like the key that we're looking for. So let's see if this refactor works. Let's remove that. Okay, so that works.

[00:07:10]
What I like about this is that it's just checking whether that key exists in the object without doing some sort of linear search through an array instead. So, I think where this is at is a good spot to introduce the actual item that we were supposed to introduce this whole time, [LAUGH] contract items.

[00:07:34]
I like where I'm at right now because as we introduce new items, now, I have a dedicated spot to add them in. And then inside update quality, we really shouldn't have to manipulate this area too much now, looking forward.