search

exercism / elixir-analyzer

GNU Affero General Public License v3.0
30 stars 32 forks source link

Analysis of Captain's Log #321

Closed BNAndras closed 2 years ago

BNAndras commented 2 years ago

In random_stardate/0, I used :rand.uniform_real/0 instead of :rand.uniform/0, but it flagged the solution as using Enum.random. I get what the analyzer is saying, but the message is confusing at first glance.

Essential Use :rand.uniform instead of Enum.random in the random_stardate function. :rand.uniform allows you to generate a random float directly.

angelikatyborska commented 2 years ago

Hmm. Indeed confusing. Using :rand.uniform_real/0 should also be an analyzer error because it never returns either 0 nor 1, and the instructions of the exercise say the lower boundary for the startdate is inclusive. We should probably add a new analyzer comment that says this.

I'm not sure what to do about the existing comment. It would be good if it could be suppressed if the new check for uniform_real finds a problem, but it's already being suppressed by another check (https://github.com/exercism/elixir-analyzer/blob/8a539ac581231c00badd94150b0d2d35791af32d/lib/elixir_analyzer/test_suite/captains_log.ex#L34) and I think multiple suppress_if aren't allowed. Is that correct @jiegillet?

jiegillet commented 2 years ago

Yes, we can only have one suppress_if, although I believe it wouldn't be very hard to allow multiple.

Another simpler way is to change the message to not mention Enum.random (or mention it only as a possible lesser method along :rand.uniform_real). Every time we assume something about the solution that we don't actually check, it comes to bite us in the butt :)