One controller and multiple repeating target or multiple controller for each area?

I decided to write a little physical inventory calculator. Nothing fancy just that I have so many cases of an item (beer) and so many loose bottles ( or full liquor bottles and percent of open).

This is done in an html table with three fields that trigger a change action. It then does a calculation and sets a total.

I first tried a single controller for the table (60+ row) and got wrapped around trying to find the targets for that row. I then put the controller on each table row and have 60+ controllers but only single targets for the row. That became very simple with less that 10 lines of code to do the calculations (different between beer and liquor)

I was just wondering if having 60+ controller is consuming to many resources, I could go back to a single controller and recalculate all rows with any change, but that also seems to be a resource consumer in that its going to come up with the same answer except for the row that changed.

Just asking for an opinion.

2 Likes

Hey salex!

Do you mind providing a little more clarity on the structure here? Are the totals per row or does each column contribute to a total for that column for all rows?

I can’t speak to the overhead of 60 controllers, but it’s obviously more than 1 controller, at least from a memory and initialization perspective. I’m wondering if one controller could be made easier by decorating each row or field with a data element that allows you to more easily find the targets you’re looking for in that row.

Probably a few ways to approach this, but any additional info or some supporting HTML/JS to illustrate the problem would be handy.

Thanks!

Thank you for your comments on my two posts. Sorry I have not replied earlier. You got me thinking on the multiple controller question and I’ve rewrote it with one controller.

This was my simple inventory calculator. The page looks like

This is not even in a database, it’s just a yaml file. I can download a ‘Quantity on hand’ csv file from my POS and I merge it with my last physical inventory. I then update each item using this calculator. The single controller is on the form of both the liquor and beer edit pages. This is a true calculator like function in that any target field that changes computes the total field, which is how many bottle of beer or shots (1.25 ounces) of liquor I have. I’m taking inventory using a iPad so each item changed checks a check box that that item was at least counted.

Again, I did the multiple controller because of trying to find the targets on each row - I figured out a way to do that!.

The new inventory_controller.js file:

/* 
  Rails.root/app/javascript/controllers/inventroy-controller.js
*/
import { Controller } from "stimulus"

export default class extends Controller {
  static targets = [ 'bottles',"wbottles", "cbottles","cases",'total','size','percent','ckd']

  connect() {
    this.idx
  }

  indexOfTargetInTargets(target,targets){
    var i
    for (i = 0; i < targets.length; i++) {
      if (targets[i] == target) {break}
    }
    return i
  }

  updateBeer(){
    var beer = event.target 
    const beerTarget = beer.dataset.target.replace('inventory.','')
    const beerTargets = eval(`this.${beerTarget}Targets`)
    this.idx = this.indexOfTargetInTargets(beer,beerTargets)
    this.totalTargets[this.idx].value = this.wbottles + this.cbottles + (this.cases * this.size )
    this.ckdTargets[this.idx].checked = true
  }

  updateLiquor(){
    var liquor =  event.target
    const liquorTarget = liquor.dataset.target.replace('inventory.','')
    const liquorTargets = eval(`this.${liquorTarget}Targets`)
    this.idx = this.indexOfTargetInTargets(liquor,liquorTargets)
    const shots = ((this.size * (this.percent / 100.0)) / 35.5)
    const bottles = ((this.bottles  * this.size) / 35.5)
    this.totalTargets[this.idx].value = Math.round(bottles + shots)
    this.ckdTargets[this.idx].checked = true
  }

  get cases(){
    return Number(this.casesTargets[this.idx].value)
  }
  get wbottles(){
    return Number(this.wbottlesTargets[this.idx].value)
  }
  get cbottles(){
    return Number(this.cbottlesTargets[this.idx].value)
  }
  get size(){
    return Number(this.sizeTargets[this.idx].value)
  }
  get bottles(){
    return Number(this.bottlesTargets[this.idx].value)
  }
  get percent(){
    return Number(this.percentTargets[this.idx].value)
  }

}

Here’s what happens

  • The connect() method defines what I guess is a global controller variable idx ( or index)
  • The updateBeer and updateLiquor functions are the only actions and are called if any input field is changed
  • I get the event target and then get the dataset.target value and strips off the controller name leaving just the target name
  • I then build a this.{name}Targets string and call eval the get the target for whatever target generated the event
  • I then get the index of the target in the Targets array by calling my function indexOfTargetInTargets(target,targets) which sets the global this.idx
  • It then does the calculation by calling the getters which use this.idx and sets the total field and checks the check box
A single controller that, while there a multiple targets, just focus on one row.

Maybe stimulus has a build in function to get the index of a target but I couldn’t find it. If there is one, someone let me know

This one was pretty simple in that I didn’t get an sum for each column. I can with this approach. Didn’t think about end-of-year inventory, but that’s simple with this single controller approach.

I reply to the other post(golf scoring) in a few days. Looking into if can use the idx scheme there.

1 Like

Nice, this looks like it does the job and is easy enough to follow along! I believe it’s possible to simplify things a little further, even. There are a few ways to approach it, but rather than searching for the index of the target each time, you could simply decorate each target with HTML (non-Stimulus) data that declares the index. That means that each form element looks something like:

<input data-target="inventory.size" data-index="1">24</input>

The resulting code (with some other changes I’ll walk through) looks like this:

import { Controller } from "stimulus"

export default class extends Controller {
  static targets = [ 'bottles',"wbottles", "cbottles","cases",'total','size','percent','ckd']


  updateBeer(evt) {
    let index = evt.target.dataset.index
    this.updateBeerRowTotal(index)
    this.checkRow(index)
  }

  updateLiquor(evt) {
    let index = evt.target.dataset.index
    this.updateLiquorRowTotal(index)
    this.checkRow(index)
  }

  updateBeerRowTotal(index) {
    this.totalTargets[index].value = this.wbottles(index) + this.cbottles(index) + (this.cases(index) * this.size(index) )
  }

  updateLiquorRowTotal(index) {
    const shots = (this.size(index) * (this.percent(index) / 100.0)) / 35.5
    const bottles = (this.bottles(index)  * this.size(index)) / 35.5
    this.totalTargets[index].value = Math.round(bottles + shots)
  }

  checkRow(index) {
    this.ckdTargets[index].checked = true
  }

  cases(index){
    return value('cases', index)
  }

  wbottles(index){
    return value('wbottles', index)
  }

  cbottles(index){
    return value('cbottles', index)
  }

  size(index){
    return value('size', index)
  }

  bottles(index){
    return value('bottles', index)
  }

  percent(index){
    return value('percent', index)
  }

  value(type, index) {
    return Number( this[`${type}Targets`][index].value )
  }
}

The total lines of code is about the same, but the event handlers no longer have to look up a target each time, they just grab the index data from the element that changed and pass that along. And there’s no need to keep an instance variable around that’s only meant to be used during the lifecycle of an event.

Now, if you wanted to take things a step further (for instance if you wanted to add a bunch of functionality to the app and thus things were about to get more complex), you could go the route of, for instance, having BeerRow and LiquorRow classes (not components) that get instantiated on initialize and encapsulate their behavior. Your two update events could become one and look like:

updateRow(evt) {
  this.rows[evt.target.dataset.index].update()
}

Now, bringing this all full circle - while the single controller approach is certainly better for memory and initialization time, I just checked Basecamp’s own site and they’re not shy about adding dozens of Stimulus components on a page, and it performs great. Thus, without doing any testing to check memory usage, I’m inclined to say that unless you’re running this on very low powered hardware you’ll have simpler, easier to maintain code by just doing one component per row :slight_smile:

One last thing - there’s no need (and it’s strongly recommended against) to use eval. Rather than:

const liquorTargets = eval(`this.${liquorTarget}Targets`)

you can (and should) do:

const liquorTargets = this[`${liquorTarget}Targets`]

Hope this was useful!

Whoa! I almost feel like the student in the Farside comic May I be excused, my brain is full.

As you can probably can tell, I’m not very proficient in javascript. You pointed out several thing I didn’t know you can do like:

const liquorTargets = this[`${liquorTarget}Targets`]

I almost made a comment in my post that I knew that eval was considered evil. But I didn’t realize you can get stuff with a string key from this.

Shortly after my post I attacked a much more complicated controller with the one controller approach. I was having trouble with the this.idx global. I also know that global are also considered evil. I finally gave up and put idx in a dataset. That fixed that problem, didn’t need my search loop. But then I ran into more problems in trying to manage two things from one place and I’m about ready to go back to the two controller approach, which you eluded to in your Now, bringing this all full circle comment.

Then there’s

wbottles(index){
    return value('wbottles', index)
  }

I found out the getter’s won’t accept a parameter and put it in the this.idx. I didn’t know there was a creature/function value() that accepts a string and an index - where is that documented?

If it’s object stuff, I’ve dabbled in it but mainly a cut-and-paste-and-hope-it-work without fully understanding objects. I guess they are about the same as ruby classes. The Stimulus documentation has about everything in it, but unless you are proficient in javascript it does not sink in. I’ve struggled with javascript since the Prototype days through the CoffeeScript/jQuery days. Guess I’ll have to find more room in my brain.

Thank you very much for your input, it was very helpful, just have to go try to put it to use like figuring out how

updateRow(evt) {
  this.rows[evt.target.dataset.index].update()
}

works!

1 Like

I’m so glad it was helpful! Yeah, Javascript is a deceptively powerful language with many intricacies (like scoping, or full grasping closures) that aren’t well understood by many. And it’s continuously evolving, which is great but requires a bit of work to keep up with.

The value() method is actually in the class, at the bottom of the code. It’s simply a convenience method to avoid repeating the same bit of code over and over :slight_smile:

value(type, index) {
  return Number( this[`${type}Targets`][index].value )
}

Keep at it and don’t hesitate to pop in with questions!