# Help with making code more effective and efficient

**URL:** <https://discourse.processing.org/t/help-with-making-code-more-effective-and-efficient/9924>\
**Category:** Coding Questions\
**Created:** [April 3, 2019, 3:21pm UTC](https://discourse.processing.org/t/help-with-making-code-more-effective-and-efficient/9924 "2019-04-03T15:21:07Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![peachman](https://avatars.discourse-cdn.com/v4/letter/p/278dde/32.png) [@peachman](https://discourse.processing.org/u/peachman)\
**Post date:** [April 3, 2019, 3:21pm UTC](https://discourse.processing.org/t/help-with-making-code-more-effective-and-efficient/9924/1 "2019-04-03T15:21:07Z")

</div>

Hi All,

First time poster, apologises in advance for bad formatting, I have only been using P5.js for about 2 weeks now. I have had previous knowledge of how code works from my programming course at college. 5 years on from that and I’ve decided to pick it back up as a hobby again and learn a new language.

I have made the below basic inventory system that you can drag and drop items from different slots to others. and drag the entire inventory by clicking on the top bar. This is a system that in my teenage years I always wanted to create but was never able to. this became my first goal for my first project.

[Inventory System Beta V1](https://editor.p5js.org/Peachman1997/sketches/zaJNgSFgj)

I have 2 main class files Inventory.js and Item.js (at the bottom of Item.js you can make Item type classes that extend off Item.js. this was mainly for me to understand extension of classes.)

the size of the inventory is all modular and you can change this on line 18 with the last 2 arguments of where the inventory object is created.

There is a function in the main sketch.js called testAddItem() where i have been adding items in the code to test the inventory… all the items are taken from a sprite map.

the project is in a state where I have been unable to get any errors. However the cases for removing too much of an item or adding past the stacklimit of an item has been handled to not break the node, but doesn’t yield any output messages to the user as this is just currently a concept.

My main question is to people who take a look at this is: Is there anything glaring obviously that I should be doing differently when coding in general?

TLDR: Made inventory System, How can i make the code better?

Thanks

Peachman

---

<div class="post-metadata">

**Author:** ![julian2](https://yyz2.discourse-cdn.com/flex036/user_avatar/discourse.processing.org/julian2/32/1551_2.png) [@julian2](https://discourse.processing.org/u/julian2)\
**Post date:** [April 4, 2019, 9:48pm UTC](https://discourse.processing.org/t/help-with-making-code-more-effective-and-efficient/9924/2 "2019-04-04T21:48:48Z")

</div>

Hi! and welcome! 😃 , just noticed that it seems you keep developing on the same linked resource, so when others go to take a look, things might not be in a working state / halfway between 2 features! (this might be discouraging for people to try and debug to get the code into a working state, and _then_ comment on the structure of your code), also when you do get suggestions, and implement the changes, others that come by to read the thread might not follow the thread as easily!

for starters, it seems like you have a fairly good grasp of coding already (you mentioned you already had some experience from 5 years ago, and it shows!), and as such, there aren’t that many _obvious_ mistakes that pop up (in contrast to those that are completely new to coding).

In general, asking for something specific is much more likely to yield answers (I just noticed that nobody answered over the last few days), and it really boils down to the vague “is there something wrong?” type of question!

## some ‘naming’ nitpicking:

```auto
  show() {
    if (this.showing) {

```

here I would maybe choose a better word than ‘show’ for your class method that does the rendering, maybe `draw` or `render` are good alternatives. Especially when you then use the word ‘showing’ in the next line, as the _state_ for whether the class is _to be drawn_ or not.

`if (this.showing) {`

I like to mirror the name `draw()` library function in my class methods (reducing the vocabulary of my program… even though `show()` is a perfectly valid name!).

another name I don’t really like is `overBar`:

```auto
if (this.overBar) {

```

it’s _inside_ the Inventory, but it _means_ that “the mouse is over the drag-bar”, so I would name it `mouseOverBar`.

another one is `locked`:

```auto
function mouseDragged() {
  if (locked) {
    inventory.dragMove();
  }
}

```

here I think it _means_ “the inventory is locked to the mouse”, so I would name it `inventoryLockedToMouse`, but I would also consider moving the variable _into_ the Inventory class, since it only seems to relate to the inventory. Once that’s done, the need for “inventory” in the variable name becomes redundant, and the name instead becomes “lockedToMouse” 🙂 .

I hope this gives a window into how I think when I go about naming things, but it’s not easy…  
It’s not without good reason when people say “the two hardest things in programming is ‘naming things’, ‘cache invalidation’, and ‘off by 1’ errors”. Naming comes with practice, and I’m by no means an expert so don’t think the names I chose are “the only right ones”, I’d instead focus on the _process_ of naming. (and it’s so easy to be opinionated about naming)

# comments

## code ‘decisions’ as comments

```auto
 //did bug here once? cannot get it go again.

```

(assuming that this comment alludes to a ‘bug that happened’ at _some_ point in time, but after a change in your code, it stopped happening)

ok, normally I wouldn’t even mention ‘git’ on this forum, as processing/p5js is mostly for ‘quick prototyping’ (and fun), but given that you’re a bit experienced already, and the vagueness of the question, I think this might be worth at least _mentioning_! but If you end up wanting to grow this application to something bigger, and maybe come back to it in the past, or share it with a friend, then ‘git’ becomes an inevitable subject/hurdle. Some type of comments just ‘disappear’ when you start writing them as ‘changes / decisions over time’, you made the decision at “some point in time”, and it belongs to “some piece of code”, and the place to write a comment linking a decision to the change is… VCS! (version control system) I called it ‘git’ earlier, but that’s just the most _common_ form of VCS out there!

another type of ‘code decision’ as comment is the famous ‘commented out code’:

```auto
     //item = new Item(itemSheet, 16, x, y, itemID - 1);
      //items.push(item);

```

They have a tendency to ‘stick around’ more than they tend to ever be uncommented again, so once you “_make the decision_” to “_not have this code execute anymore_”, just delete it, and let the code exist in your VCS!

## comments as code (aka. “repeating what the code does”)

comments such as:

```auto
 //draw rectangle top bar

```

should be “_written as code_”, if a function is becoming too long to ‘read easily’, and feel like they necessitate a comment to describe what it’s doing, it’s very likely that the function itself has become _too large_, and is doing too many things, here I would split this function up into the following:

```auto
drawTopBar() {
   ...
}

drawItemCells() {
  ...
}

drawInventory() {
  this.drawTopBar();
  this.drawItemCells();
}

```

It might seem like “extra code I have to write, and the comment is just 1 line, whereas a function definition and call requires at _least_ 4 lines!..”, but wait! there _are_ some benefits!

1. code never gets “outdated”. Sometimes a comment is left behind when the code changed, and it leads to _more_ confusion than if it hadn’t been there to begin with!
2. it’s easier to “see the scope” of related code, the code that “draws the top bar” is separate from code that “draws the inventory”
3. code is never _wrong_, or _lies_ to you, it simply _does_
4. code is interactive! (you can run it and see what happens, and step through it with a debugger)
5. refactoring becomes easier, swap out the whole function, or move the function somewhere else! (this point kinda relates to the point about “scope”)

# standing on the shoulders of giants (#storytime)

Now yes, I just wanted to use that phrase in the title ;), but we really do “stand on the shoulders of giants” when it comes to code (as with most things), languages come with **standard functions** , thousands of **libraries** simply _exist_, **conventions** are formed! (and subsequently not followed!), but it’s a wonderful place to be!

I mention this because I struggle with this myself (still), but at some point on my programmer journey I told myself in a sort of strict ‘finger wagging’ way: “spend a _lot_ of time researching your first moves!, it’s not worth it to start writing code too early!”, I fondly remember going off and wanting to “write code!”, and sure I knew how to apply the knowledge I had _just_ learnt from university, and went off creating stuff since “this is what I was trained to do!.. writing code!”

Not once did I stop to think “… what _is_ out there?!.. what do _other_ people do?..” (the worst part about this story is that I sometimes _STILL_ do this!, despite having supposedly “learnt my lesson”).

to make this story relevant here, when you have code like this `addItem` (and also `removeItem`):

```auto
addItem(item, quantity) {
    //print("ADDITEM")
    this.item = item;
    var added = false;
    //if item is already in the inventory, add to its quantity.
    for (var i = 0; i < this.inventoryCell.length; i++) {
      if (this.inventoryCell[i][2]) { //if there is something in cell
        if (this.inventoryCell[i][3].ID === this.item.ID) {
          this.inventoryCell[i][3].incimentStack(quantity); //this does return a true of false depending on if the stack was met.
          added = true;
          break;
        }
      }
    }
    //if the item was not in the inventory, add it to the next empty slot.
    if (!added) {
      for (i = 0; i < this.inventoryCell.length; i++) {
        if (this.inventoryCell[i][2] === false) { //in the cell is empty
          this.inventoryCell[i][2] = true; //cell is now full
          this.inventoryCell[i][3] = this.item; //we set index 3 of the cell to the item.
          this.inventoryCell[i][3].incimentStack(quantity) //and we then incriment the item by that amount.
          added = true;
          break;
        }
      }
    }
  }

```

It might be worth noting that Javascript has some good standard Array functions you can take advantage of, such as [find()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find), [indexOf()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf), and [findIndex()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex). The last one (`findIndex()`) might come in real handy if you had an array of items, and you wanted to iterate through them to find a specific one! and if you restructured your 2D array to be just a single array of data, you’ll find that you might be able to cut down on a large amount of code this way. (all that 2D array iteration, and 2D array access code)

In this case, you can separate the way you want something to **_look_** (a 2D grid of items), from the way you want to **_represent_** it in code (a single array of items). This is often hard to do on the fly, and perhaps easier in retrospect. But in general it can be worth thinking about these 2 things _separately_, and giving them an equal share of thought! (they’re both important, and a change in one can make the entire problem a _lot_ simpler).

Now I’m not saying that doing this refactoring to a 1D array is “_the right way_” to go for this code (in fact, it might turn out to be complicated for both drawing & mouse interaction, going both from 2d -\> 1d (mouse click coordinates), and 1d -\> 2d (drawing cell from index to canvas coordinates)), but it’s the _thought_ that counts! (and the incentive is “how can I utilize existing functions like [findIndex()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findIndex) here?”)

---

<div class="post-metadata">

**Author:** ![peachman](https://avatars.discourse-cdn.com/v4/letter/p/278dde/32.png) [@peachman](https://discourse.processing.org/u/peachman)\
**Post date:** [April 5, 2019, 6:32pm UTC](https://discourse.processing.org/t/help-with-making-code-more-effective-and-efficient/9924/3 "2019-04-05T18:32:55Z")

</div>

HI Julian,

Thanks for taking the time to have a look at this small project of mine, it really means a lot. I will definitely take this advice on board. I have parked this inventory project for the moment to start on something new that i want to pull together with this and other projects to simply learn different aspects of coding., But will come back to this later this week (got some holiday this week to hopefully learn some more!)

I would like to mention about the comments. They’re just comments!!! But all seriously though, I didn’t realise how important they are. i think this might be because of the lack of confidence i have when coding, as well as general just house keeping of the code.

When i was coding the addItem and removeItem, i was thinking that there must have been a better way of doing it. I will look into trying to use the array functions you said to try and reconstruct this section (will be the 3rd time now but that’s how we learn!)

Thanks again for the feedback! this was exactly what I was looking for!

Thanks

Ryan

---

<div class="post-metadata">

**Author:** ![julian2](https://yyz2.discourse-cdn.com/flex036/user_avatar/discourse.processing.org/julian2/32/1551_2.png) [@julian2](https://discourse.processing.org/u/julian2)\
**Post date:** [April 5, 2019, 11:05pm UTC](https://discourse.processing.org/t/help-with-making-code-more-effective-and-efficient/9924/4 "2019-04-05T23:05:41Z")

</div>

Glad it was helpful! I was worried about the quality of my post, whether it made any sense, was easy to read… etc… and I know it was probably way too lengthy (I wrote it pretty late that day, and… I seem to always browse this forum past midnight… 🌙 ).

* * *

> I have parked this inventory project for the moment to start on something new that i want to pull together with this and other projects to simply learn different aspects of coding.

Ultimately coding _is_ about scrapping projects! (and _maybe_ coming back to them later!)

* * *

> When i was coding the addItem and removeItem, i was thinking that there must have been a better way of doing it.

Yeah! the subconscious is usually a _really_ good indicator for when something is due for a refactor, or when to start looking for a better solution. Sometimes you have to deal with a sub-par solution there and then, but it often comes back to you sometime later, maybe after waking up the next day (always keep a notebook ready!).