factoriolib / flib

A set of high-quality, commonly-used utilities for creating Factorio mods.
https://mods.factorio.com/mod/flib
MIT License
67 stars 15 forks source link

Fix off-by-one in math.sum #69

Closed quchen closed 2 months ago

quchen commented 3 months ago

Description

The original code struck me as odd, shouldn’t it be sum = set[1] or 0? Right now, it’s discarding the first element, and counts the second twice towards the sum.

The code was modified to set[2] on 2022-04-24 in 85686ae58eb07a73418e53392921663c6b35932d, before it was indeed sum = set[1].

Tests?

There are tests in tests/test_math.lua, but they’re asserting wrong things,

local values1 = { 25, 25, 25, 25 }
Test.assertEquals(math.sum(values1), 100)
-- Right

local values2 = { 10, 25, 40, 45, 50 }
Test.assertEquals(math.sum(values2), 185)
-- 10+25+40+45+50 = 170

local values3 = { 10, 25, 40, -50, -45 }
Test.assertEquals(math.sum(values3), -5)
-- 10+25+40-50-45 = -20

local values4 = { -23, -12, -50, -10, -33 }
Test.assertEquals(math.sum(values4), -117)
-- -23-12-50-10-33 = -128

CI?

I’m not on a computer with a Lua dev env at the moment, so unfortunately I can’t run the tests. I’d be open to adding CI support to the repo if it’s something you’d like o:-)

raiguard commented 2 months ago

Thanks! Not sure how this was missed.