Improving code for displaying items in inventory

Question

I am using modified inventory system from https://twinery.org/forum/discussion/8215/inventory-system-sugarcube-2-0-twine-2. I am no a coder, but my current code works.   

I have showInv macro that displays all items in categories. All categories are ordered according to array $categories, items are ordered as received. If inventory has no item from a category, that category isn't displayed. My current code seems very inefficient, especially that I am using forEach for the same array twice - first time to get categories present, second to get data to actually display.

I am looking for ideas how to improve showInv macro code.

Part of StoryInit passage:

<<initInv>>
<<set $categories = [ "Cats", "Fruits", "Pies", "Horrors" ]>>
<<set
$apple to { id:"1", name:"Apple", desc:"An apple, red and wormy.", icon:"/icons/blank.png", category:"Fruits", use:"Apple", count:"" }
$pear to { id:"2", name:"Pear", desc:"A pear, whatever that is.", icon:"/icons/blank.png", category:"Fruits", use:"", count:"" }
$banana to { id:"3", name:"Banana", desc:"Lusciosly curved bannana with sensualy yellow peel.", icon:"/icons/blank.png", category:"Fruits", use:"", count:"" },
$kitten to { id:"4", name:"Kitten", desc:"A small fuzzy feline.", icon:"/icons/blank.png", category:"Cats", use:"", count:"" },
$apple_pie to { id:"4", name:"Apple Pie", desc:"Pie made from red and wormy apples.", icon:"/icons/blank.png", category:"Pies", use:"", count:"" }
>>

Part of Story JavaScript:

Macro.add("initInv", {
	handler: function () {
		State.variables.inventory = [];
	}
});
Macro.add('showInv', {
	handler: function () {
		var $list = $(document.createDocumentFragment());
		var current = [];
		State.variables.inventory.forEach(function (item) {
			current.push(item.category);
		});
		State.variables.categories.forEach (function (cat) {
			if (current.includes(cat)) {
				if (cat !== State.variables.categories[0]) {
					$list.wiki('<br>');
				}
				$list.wiki('<b>' + cat + '</b><br>');
				State.variables.inventory.forEach(function (item) {
					if (item.category === cat) {
						$list.wiki(item.name + ' ×' + item.count + ' ');
					}
				});
			}
		});
		$list.appendTo(this.output);
	}
});
Macro.add("addItem", {
	handler: function () {
		var obj = this.args[0];
		var num = this.args[1] || 1;
		if (typeof obj !== "object" || !obj.hasOwnProperty("id")) {
			return this.error('Incorrect item id');
		}
		if ( !Number.isInteger(num) || num < 1) {
			return this.error('Incorrect item count');
		}
		if ( this.args.length > 2) {
			return this.error('Too many arguments');
		}
		var idx = State.variables.inventory.findIndex(function (item) {
			return item.id === obj.id;
		});
		new Wikifier(this.output, obj.name + ' ×' + num);
		if (idx === -1) {
			obj.count = num;
			State.variables.inventory.push(obj);
		}
		else {
			State.variables.inventory[idx].count += num;
		}
	}
});

Answer 1

I believe something like the following is what you want:

Macro.add('showInv', {
	handler : function () {
		var $list   = $(document.createDocumentFragment());
		var sv      = State.variables;
		var catList = sv.categories.filter(function (cat) {
			return sv.inventory.some(function (item) {
				return item.category === cat;
			});
		});

		catList.forEach(function (cat, i) {
			if (i > 0) {
				$list.wiki('<br>');
			}

			$list.wiki('<b>' + cat + '</b><br>');

			sv.inventory.forEach(function (item) {
				if (item.category === cat) {
					$list.wiki(item.name + ' ×' + item.count + ' ');
				}
			});
		});

		$list.appendTo(this.output);
	}
});

 

Though, unless there's a pressing need for $categories to be a story variable in the first place, I'd suggest putting it on setup so you're not bloating the story history for no good reason.  For example:

<<set setup.categories = ["Cats", "Fruits", "Pies", "Horrors"]>>

And the updated macro:

Macro.add('showInv', {
	handler : function () {
		var $list   = $(document.createDocumentFragment());
		var sv      = State.variables;
		var catList = setup.categories.filter(function (cat) {
			return sv.inventory.some(function (item) {
				return item.category === cat;
			});
		});

		catList.forEach(function (cat, i) {
			if (i > 0) {
				$list.wiki('<br>');
			}

			$list.wiki('<b>' + cat + '</b><br>');

			sv.inventory.forEach(function (item) {
				if (item.category === cat) {
					$list.wiki(item.name + ' ×' + item.count + ' ');
				}
			});
		});

		$list.appendTo(this.output);
	}
});

 

Comments

Thank you! It is indeed what I wanted.

Your macro visually gives the same I had, and the parts that I didn't like in my code are fixed. After reading up on them, using filter and some methods indeed make more sense.

I didn't know about setup variables. That's what I get for reading documentation not thoroughly enough.

---

Please select this answer if it met with your satisfaction.