Export SetCell and GetCell.

[adlerjohn]

Fixes #83
Fixes #89

Unlike the suggestion in #83 to not reset cached roots, that functionality was kept. The reason is that setting a cell from nil to non-nil does not guarantee that the roots match up with the leaves. The roots actually need to be re-computed to guarantee this, so the cache is cleared every time a cell is set.

[codecov]

Codecov Report

Merging #92 (3910c4f) into master (80b016d) will decrease coverage by 0.84%.
The diff coverage is 25.00%.

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
- Coverage   79.54%   78.69%   -0.85%     
==========================================
  Files           8        8              
  Lines         435      446      +11     
==========================================
+ Hits          346      351       +5     
- Misses         54       60       +6     
  Partials       35       35              

Impacted Files	Coverage Δ
datasquare.go	88.32% <14.28%> (-4.05%)	⬇️
extendeddatacrossword.go	75.94% <100.00%> (ø)
leopard.go	100.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80b016d...3910c4f. Read the comment docs.

[liamsi]

Change looks good but did make we sure that with making the two of these public, we did not make the API less fool-proof (or easier to shoot yourself in the foot)?

[Wondertan]

Why is this breaking @adlerjohn? It just adds new features to the API without breaking anything existing.

[Wondertan]

Unlike the suggestion in #83 to not reset cached roots, that functionality was kept. The reason is that setting a cell from nil to non-nil does not guarantee that the roots match up with the leaves. The roots actually need to be re-computed to guarantee this, so the cache is cleared every time a cell is set.

This omits the #84 from the picture. The #84 could be explained as "set expected roots once and ensure reconstruction matches them" :

• The cached roots are not the ones we compute inside the square but those we get from the square setup by the user. (I also mixed up those two while describing issues).
  • We have roots from fixed DAH, and we just want to ensure the square we reconstruct matches it.
  • We don't have new roots for each new Repair attempt, while the current API asks you to pass those each attempt. Or is there a use case where one square is repairable by different roots?
  • Maybe this also only makes sense with the custom constructor(api: Consider dropping EDS importing in favour of EDS constructor #85) only.
• I agree resetting computed roots is right even if we allow setting only non-nil shares. It does not make sense to reset the expected roots, which I mixed up with the cached/computed.
• It still makes sense, though, to disallow setting(fool-prooving) non-nil cells to prevent computed, verified, or already set by the user cells from changing (within this PR).

[adlerjohn]

did make we sure that with making the two of these public, we did not make the API less fool-proof (or easier to shoot yourself in the foot)?

I don't see why it would be less foolproof. The alternative is setting cells manually then re-importing the data square, which is way more prone to screwing up.

Why is this breaking

Ah you're correct, the public API is only additive.

I agree resetting computed roots is right even if we allow setting only non-nil shares.

Yep, the resetting done here is for the computed roots.

It still makes sense, though, to disallow setting(fool-prooving) non-nil cells to prevent computed, verified, or already set by the user cells from changing (within this PR).

That would make tests more cumbersome without an additional method to allow setting a cell to nil, but that can just be added!

[adlerjohn]

@Wondertan fixed in 3910c4f

[Wondertan]

Nice. Thanks.