Error in Sum of Multiples mentoring suggestion

In Sum of Multiples Python, the mentoring help suggests this solution:

``````return sum({n for m in multiples for n in range(m, limit, m)})
``````

This solution doesn’t work, as `m` might be `0`, in which case `range` would raise an error. I’ve tried running this code and two of the tests fail.

The correct version would be:

``````return sum({n for m in multiples if m for n in range(m, limit, m)})
``````

… or more explicitly:

``````return sum({n for m in multiples if m == 0 for n in range(m, limit, m)})
``````

This is the proposed change on GitHub.

1 Like

Please don’t `if number`. It is morally wrong. Please just say what you mean: `if m != 0`.

Other than that:

( Though I’m slightly in favor of `range(0, limit, m)`, for reasons of morality. )

To be clear, this is an opinion and worth stating as one rather than as fact

Different languages have different attitudes towards questions of truthiness. And we’re coding here, not doing maths, so different approaches/attitudes/rules apply.

Yes, it is a personal preference. Its technical correctness is obvious. (In Python anyway – I hear in Ruby it would be technically wrong.)

While the linked paper is about the term as used in the field of mathematics, I intend the same sense but also outside of mathematics.

As for Python’s attitude specifically: I am not aware of a generally held preference for treating numbers as booleans, but did expect myself to know about such had there existed any. So I think there is instead a general preference for not implicitly converting numbers into `bool`s. The reverse is not true: implicit treatment of `bool`eans as numbers is somewhat common.

I’m not sure about the Python preference, but I generally just find this more succinct. The Zen of Python:

Beautiful is better than ugly.

However, also:

Explicit is better than implicit.

So I’m unclear, and cool with both .

As a counter point, the Google Style Guide says,

Use the “implicit” false if at all possible.

To address the actual topic, that diff looks good to me!

I too have seen this usage often in Python code, but `n != 0` is fairly common too, due to it’s more general popularity (not available in many other languages).

That Google Style Guide also says

Also, taken literally the advice «Use the “implicit” false if at all possible.» incurs an obligation to search docs for opportunities to use implicit falsiness. There really are/were nonobvious cases.

Explicit is always better. Being implicit opens up the possibility of bugs. I’ve seen this happening multiple times.

That’s true in real-life cases or even in other theoretical cases, but here, as Exercism is an educational platform first and foremost, teaching students concise, idiomatic, solutions (perhaps with a warning about the potential disadvantages?) is best, I feel. Especially as it cannot not be a number in this case.

Explicit is always better. Being implicit opens up the possibility of bugs. I’ve seen this happening multiple times.

Absolutism aside, I’ve read that `raku` deliberately embraces messiness. The idea being that on the other end of that spectrum lies an expressivity and clarity that can reach beyond that achievable in languages otherwise so constrained.

1 Like

Meta-discussion note. This is a topic that’s been popping up a lot for me lately, in Liz Wiseman’s book, “Multipliers” and Ned Batchelder’s PyCon keynote presentation. This is definitely something I’m guilty of myself.

It’s easy to have strong opinions and state them as facts. However, stating them as fact makes it challenging to have a conversation. Rather than inviting a discussion, it states that “this is fact and there is nothing to discuss here”.

I’ll try to remember to link Ned’s talk once they have it uploaded. It’s a great watch about interacting with the rather complex and unpredictable systems known as people.

1 Like

`bools` are defined as a subtype of `int` in Python. See PEP 0285 as well as the Python 3.11 docs. In numeric contexts `bool`s behave as 0 (False) and 1 (True) Python 3.11 Boolean Values.

The ‘gotcha’ described in the Google style guide is for situations where it is unclear if some other ‘truthy’ or ‘falsey’ value is going to be involved, and accidentally treated as an `int`. If that’s the case, it is better to be explicit, so that you don’t say, overcount your zero/False values because your function got `None` in the dataset.

I added a comment to the PR. The style for the Python track is to not use single letter variable names - not even in comprehensions. They’re hard to read/understand when someone is getting familiar with the comprehension syntax (and often even when they are familiar with the syntax).

Yes - we have a lot of examples that do use them, and eventually I would like to clean those up. And yes - people are free to use whatever they want in their code - but I would like docs and examples on the track to be more conservative.

1 Like

`bool` being a subtype of `int` does not come into treating `int`s as `bool`eans though. It does the other way around, i.e. treating `bool`eans as numbers (which they ‘are’), but that is not what that quote of me is about. (But the last sentence of that paragraph is.)

Anyway, I re-read the relevant section, namely

…and now I am confused: how could one ever accidentally handle `None` as `0`? (I do see how one might mishandle either or both as `False`, but that is a different question.) Numerical manipulations are out: those are type/runtime errors, not logical ones.

Also: this text warns about confusing types, but then immediately greenlights comparing against `0` specifically when the types are very clear instead. Perhaps they meant not known instead of known?

Very contrived, but theoretically I have a int-like class with a modulus-like operator:

``````from typing import Optional

class Foo(int):
def __mod__(self, other) -> Optional[int]:
if self == 3:
return None
return super().__mod__(other)

for v in (1, 10, 3):
foo = Foo(v)
if not foo % 10:
print(f"Implicit: {foo} is a multiple of 10.")

for v in (1, 10, 3):
foo = Foo(v)
if foo % 10 == 0:
print(f"Explicit: {foo} is a multiple of 10.")

====

» python a.py
Implicit: 10 is a multiple of 10.
Implicit: 3 is a multiple of 10.
Explicit: 10 is a multiple of 10.
``````

Overriding `%` isn’t that odd; `pathlib.Path` overrides `/`.

I thought it was fine to use single letter names in comprehension and even told so in a mentoring discussion . Thanks for correcting it!

Apart from this, this line in the same file:

Common Suggestions

• It’s tempting to write a complex one liner for this, but that should be discouraged in favour of PEP8 compliant solutions for readability and maintainability.

Huh? The one-liner doesn’t seem complex to me.

Note, the Python track’s style isn’t necessarily identical to other the style followed in other Python codebases. It might be fine in some code bases and not in others. Styles are not universal. Pylint, by default, will complain about single character variables in broader scopes but not in one liner comprehensions.

I agree this particular one liner isn’t very complex. Some solutions may use much more complex statements.

I agree this particular one liner isn’t very complex. Some solutions may use much more complex statements.

True, but this section isn’t universal - that is, I haven’t seen it in all mentoring help documents, or even many. It’s doubtless applicable to other exercises, but shouldn’t we remove it here?