Fixed a broken payment test (+ another small fix)

[dwardu]

Test was referencing a local variable instead of calling the contract method.

[dwardu]

As a sidenote, I believe the public getter payee(uint256) should be renamed payees(uint256).

The standard way to implement this would have been to declare a public variable payees as follows:

address[] public payees;

and then let the payees(uint256) getter be generated automatically.

But I can see that the implementation is as follows:

address[] private _payees;
function payee(uint256 index) public view returns(address) {
  return _payees[index];
}

I gather that this was done so that contract variables could be _prefixed, but the public getter would be “clean”. However I think that at least the public getter should be named as it would have been named if there weren’t this “workaround.”

[nventuro]

Awesome @dwardu, thanks! It's a bit worrying that this didn't cause an error in the suite, I'll review the other tests.

[nventuro]

Good catch!

[dwardu]

I caught the mistake in the PR template… because I was obeying the PR template 😀

[nventuro]

Regarding the naming of the getter, I think it was @ElOpio who made that call, and I agree with it. payees makes sense for an array variable, but payees(index) feels a bit weird to me, kind of looks like you're retrieving many of them.

[dwardu]

Yes, despite payees[index] not feeling bad, payees(index) feels strange… but that’s how Solidity does it for public getters… I have no issue with it, it was just a sidenote.

[dwardu]

Yes, I was also wondering how the failed test had managed to go through!

[nventuro]

I think the fact that we didn't get an error was related to the promises not being awaited, Mocha sometimes does weird stuff in those cases.

Anyway, thanks a lot!

[nventuro]

Btw, regarding your naming suggestion, feel free to open an issue for us to discuss this, if you think it may be a source of confusion :)