On 08/09/2019 19:34:33, Chris Angelico wrote:
On Mon, Sep 9, 2019 at 4:13 AM Rob Cliffe via Python-ideas
<[email protected]> wrote:
On 07/09/2019 18:59:49, Chris Angelico wrote:
On Sat, Sep 7, 2019 at 11:27 PM Rob Cliffe via Python-ideas
<[email protected]> wrote:
A chance for me to bang the drum on one of my pet themes:
Sometimes the readability of code is improved by breaking the sacred
taboo of 1 statement per line, if it allows similar constructs to be
vertically aligned:
if max_results > 0 : query['max_results'] = max_results
if active is not None : query['active'] = active
if deleted is not None: query['deleted'] = deleted
If this comes out ragged in your browser, the colons and equal signs are meant
to be vertically aligned.
I'm not bothered by putting an if and a simple statement together, but
I am definitely bothered by the need to vertically align them, and
especially by the triple repeated name. Not a fan of this style -
looks like a bug magnet and maintenance burden.
ChrisA
The OP's code:
if max_results > 0:
query['max_results'] = max_results
if active is not None:
query['active'] = active
if deleted is not None:
query['deleted'] = deleted
is perfectly reasonable and has the same triple repeated name. Would you
rewrite it differently, and if so, how?
I'm just saying that aligning similar elements and bringing them
together, without intervening lines, makes it
easier to see the code structure and spot typos. Not to mention using
less vertical screen space.
I don't understand why you think it is a bug magnet and maintenance
burden. I think it is *easier* to maintain:
in which of the following is it easier to spot the mistake?
# Version 1:
if max_results > 0 : querydata['max_results'] = max_results
if active is not None : querydata['active'] = active
if deleted is not None: quervdata['deleted'] = deleted
# Version 2:
if max_results > 0:
querydata['max_results'] = max_results
if active is not None:
querydata['active'] = active
if deleted is not None:
quervdata['deleted'] = deleted
Data point: I have used this style repeatedly in my code (only where
appropriate, of course) and find it helpful.
Now add a fourth one, for result_count. What does your diff look like?
Do you have to edit every other line just to realign it? That's the
maintenance burden - fiddling around with formatting.
Honestly, I couldn't see the mistake in _either_ version until I
looked pretty closely, upon which it was equally visible in both. If I
were using this in a full app, it would be something a linter could
easily spot, again regardless of the alignment.
I should have used a more obvious typo than changing a "y" into a "v",
they look too similar:
if max_results > 0 : querydata['max_results'] = max_results
if active is not None : querydata['active'] = active
if deleted is not None: querodata['deleted'] = deleted
Can you honestly say it's not easier to spot the inconsistency when elements
that should be identical are adjacent?
(BTW I changed the identifier from query to querydata because my
spell-checker,
and maybe yours, highlights the mistake instantly in both versions.)
Rob Cliffe
You're right that the triple repeated name is not caused by your
style. However, your suggestion also doesn't solve it. What I'd
probably want to do, here, is build the dictionary with everything,
and then have a separate pass that removes anything that's None (or
maybe "any of these keys, if the value is None"), and then just
special-case the requirement for max_results to be positive. But I'd
be looking to make a broader change somewhere, taking into account a
lot more code, rather than doing this whole "take local names and
plonk them into a dict" thing at all.
ChrisA
_______________________________________________
Python-ideas mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-ideas.python.org/
Message archived at
https://mail.python.org/archives/list/[email protected]/message/65SSURULSQU27F2KVUYCESWAWUTRNB4O/
Code of Conduct: http://python.org/psf/codeofconduct/
---
This email has been checked for viruses by AVG.
https://www.avg.com
_______________________________________________
Python-ideas mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-ideas.python.org/
Message archived at
https://mail.python.org/archives/list/[email protected]/message/TCE7NZQABDD7DMH5G7MILIIFZMVE3V2M/
Code of Conduct: http://python.org/psf/codeofconduct/
