`for(let unit in ...) { unit.quantity`

[cshaa]

Hey Eric! I've been reading through the code of make-things-less-complicated and there's a part that didn't make sense to me. On line 384 of Unit.ts there is this code:

// Did not find a matching unit in the system
// 4. Build a representation from the base units of all defined units
for (let dim in result.dimension) {
  if (Math.abs(result.dimension[dim] || 0) > 1e-12) {
    let found = false
    for (let unit in unitStore.defs.units) {
      if (unit.quantity === dim) {
        // TODO: Try to use a matching unit from the specified system, instead of the base unit that was just found
        proposedUnitList.push({
          unit,
          prefix: unit.basePrefix || '',
          power: result.dimension[dim]
        })
        found = true
        break
      }
    }
    if (!found) ok = false
  }
}

The for..in loops define variables dim and unit as the keys of an object, right? Then how could unit.quantity and unit.basePrefix be anything other than undefined? Did you mean something like this instead?

for (let unit of Object.values(unitStore.defs.units)) {

The Object.values function returns the values of enumerable properties of an object.

[ericman314]

Ah, good catch! Actually we still need the property name unit, so maybe:

// Did not find a matching unit in the system
// 4. Build a representation from the base units of all defined units
for (let dim in result.dimension) {
  if (Math.abs(result.dimension[dim] || 0) > 1e-12) {
    let found = false
    for (let unit in unitStore.defs.units) {
      if (unitStore.defs.units[unit].quantity === dim) {
        // TODO: Try to use a matching unit from the specified system, instead of the base unit that was just found
        proposedUnitList.push({
          unit,
          prefix: unitStore.defs.units[unit].basePrefix || '',
          power: result.dimension[dim]
        })
        found = true
        break
      }
    }
    if (!found) ok = false
  }
}

The variable naming here is really bad. There are a few places where we have unit.unit.name and other ambiguous things like that. And then in this loop, unit is a string. I want to de-duplicate variable names like that--porting to typescript is probably the right time to do that.

[ericman314]

No, you are right. Disregard the code in my previous comment. proposedUnitList[].unit is supposed to be an object, not a string. I guess we really do need those type definitions!