4 ms·

From current production code: class VendorOrderShipmentResource(): ... def post(...): ... for item in data.order_items:

by pyre 11y ago

From current production code: class VendorOrderShipmentResource(): ... def post(...): ... for item in data.order_items: ... if not shipment_qty: abort(400, message='All order_items must have a quantity_to_ship.') That line goes out to col 86. I realize that it's possible to break it like this (#1): abort( 400, message='All order_items must have a quantity_to_ship.') or this (#2): abort( 400, message='All order_items must have a quantity_to_ship.' ) or this (#3): msg = 'All order_items must have a quantity_to_ship.' abort(400, message=msg) or this (#4): abort(400, message='All order_items must have a ' 'quantity_to_ship') I'm not really a fan of any of those: 1. I'm not a fan of this one, in this case at least, because it "looks weird" due to the mismatch between the length of the function name, and the length of the next line. Also, I'm not a fan of "hiding" the closing parenthesis at the end of the line, rather than putting it on new line at the original indent level (like #2). 2. I'm not really a fan of this because the first argument and the function name are so much shorter than the message argument, that it looks really lopsided. 3. I'm not a fan of this because it seems stupid to create a new variable just to use it on the next line (and never again). 4. I'm not really a fan of breaking strings up this way, but sometimes I'm forced to. Breaking strings up this way tends to be really fragile when you want to modify them, and might need to reflow the entire wrap. Personally, I'm just a fan of increasing line length to ~100. The number of things that go over 100 columns (in practice) is much smaller than the number of things that go over 80 columns.

sago 11y ago
Thank you. Why is 3 stupid? That would be my preference. It is clear, simple and gets compiled away (in many cases). I alias in variables frequently, to make things more explicit. For example, I might write iterations = 300 for _ in range(iterations): ... which makes it less likely I'll end up repeating things or copy and pasting when new cases come up, e.g. if not shipment_qty: abort(400, message) elif shipment_qty < 0: abort(400, message) What happens in your case if you come back and edit the message and add a few characters, exceeding your longer line length? When I'm forced to break a string it is usually an indication that I need a better way of defining text than inline in expressions in code.
- pyre 11y ago
  > Why is 3 stupid? Just to be clear, I do use that pattern in practice, but I usually feel 'dirty' about it. Sometimes I also do things like this: is_admin = (membership.type == 'master') is_current_group = (membership.group == session.group) if is_admin or is_current_group: do_something() but I find this easier to justify because I'm adding something more than just the breakup of a line. I'm adding semantic meaning to other developers reading the code. In the example I gave, saying "message=msg" doesn't add anything (as opposed to say "message=bad_shipment_msg", but maybe the definition of "bad_shipment_msg" pushes the string past 80 columns by itself). As for the example of reusing the message, maybe it's bad practice, but I would end up with tailored messages describing the edge case (e.g.): if not shipment_qty: abort(400, message='Must pass a quantity') elif shipment_qty < 0: abort(400, message='Quantity must be > 0') Edit: I do the boolean definition thing more then "sometimes" because I prefer to make complex boolean operations easy to understand, and to also convey the meaning of what you want to achieve.
  - sago 11y ago
    I think it is pretty essential for boolean expressions too, yes, anything that isn't a single comparison, at least. Interesting you find aliasing in variables 'dirty'. Would you feel dirty writing something like def my_method(self): contents = self.contents ... etc ... which I use often if I'm going to use self.contents a lot in that method. It has some performance benefit in rare cases, but mostly it reduces clutter, makes lines smaller, and means I type less. But again, it is 'pointless', in one sense. From the current code open on my machine, even one character variable names!: w, h = self.width, self.height ... Dirty? Stupid? (I'm not being facetious, I'm genuinely interested in your aesthetic sense here).
    - coldtea 11y ago
      With autocomplete you don't get anything with "one character var names" (leave those for i, j, k etc). And no reason to pollute the local namespace with both self.contents and contents. What if you do this somewhere lower for example: contents = contents[:limit] did you mean to NOT change self.contents? Because that's what you got. Whereas: self.contents = self.contents[:limit] would propagate it.
tedunangst 11y ago
Regarding your last point, it's iteratively true. The number of things that go over 120 is much less than the number that go over 100. One always seems to need just a little more space.
- pyre 11y ago
  I wasn't very clear, but I meant to say that the number of lines that will naturally end up between 80 ~ 100 columns is greater (in practice) than the number of lines that will go over 100 columns. I.e. I think that 100 is a better "sweet spot" than 80 or 120.
rtpg 11y ago
In my case I am a fan of: abort(400, message='All order_items must have a quantity_to_ship.') (not giving the parens an entire line) I'm not a fan of 80 characters, but I am a fan of less than 120 characters, if simply that there is a point where a line won't fit in a terminal thats in half of a "standard laptop" screen (i.e. the limit to where I could put two files next to each other)
s_kilk 11y ago
This is why I dislike Pythons standard of using four spaces for indentation. By the time you've declared a class, a method and a for-loop and a conditional you're already 16 characters down and struggling to fit code into the remaining space.
kevin_thibedeau 11y ago
Just let the line run long. These are only guidelines. There is inherent wiggle room. I break lines that feel too long and casually ignore the 80 column margin. Inserting unnatural breaks is just as bad as excessive line length.
- pyre 11y ago
  It's different if you're using a tool (e.g. pep8) to ensure coding style. You can tell it to ignore long lines, or you can configure it to have a "better" line length than 80. Telling it to ignore all long lines means absurdly long lines are accepted too, but configuring it for a particular number makes it a hard limit. The best option is to pick a number that is a good trade-off.

by pyre 11y ago

sago 11y ago

Thank you. Why is 3 stupid? That would be my preference. It is clear, simple and gets compiled away (in many cases). I alias in variables frequently, to make things more explicit. For example, I might write iterations = 300 for _ in range(iterations): ... which makes it less likely I'll end up repeating things or copy and pasting when new cases come up, e.g. if not shipment_qty: abort(400, message) elif shipment_qty < 0: abort(400, message) What happens in your case if you come back and edit the message and add a few characters, exceeding your longer line length? When I'm forced to break a string it is usually an indication that I need a better way of defining text than inline in expressions in code.

pyre 11y ago
> Why is 3 stupid? Just to be clear, I do use that pattern in practice, but I usually feel 'dirty' about it. Sometimes I also do things like this: is_admin = (membership.type == 'master') is_current_group = (membership.group == session.group) if is_admin or is_current_group: do_something() but I find this easier to justify because I'm adding something more than just the breakup of a line. I'm adding semantic meaning to other developers reading the code. In the example I gave, saying "message=msg" doesn't add anything (as opposed to say "message=bad_shipment_msg", but maybe the definition of "bad_shipment_msg" pushes the string past 80 columns by itself). As for the example of reusing the message, maybe it's bad practice, but I would end up with tailored messages describing the edge case (e.g.): if not shipment_qty: abort(400, message='Must pass a quantity') elif shipment_qty < 0: abort(400, message='Quantity must be > 0') Edit: I do the boolean definition thing more then "sometimes" because I prefer to make complex boolean operations easy to understand, and to also convey the meaning of what you want to achieve.
- sago 11y ago
  I think it is pretty essential for boolean expressions too, yes, anything that isn't a single comparison, at least. Interesting you find aliasing in variables 'dirty'. Would you feel dirty writing something like def my_method(self): contents = self.contents ... etc ... which I use often if I'm going to use self.contents a lot in that method. It has some performance benefit in rare cases, but mostly it reduces clutter, makes lines smaller, and means I type less. But again, it is 'pointless', in one sense. From the current code open on my machine, even one character variable names!: w, h = self.width, self.height ... Dirty? Stupid? (I'm not being facetious, I'm genuinely interested in your aesthetic sense here).
  - coldtea 11y ago
    With autocomplete you don't get anything with "one character var names" (leave those for i, j, k etc). And no reason to pollute the local namespace with both self.contents and contents. What if you do this somewhere lower for example: contents = contents[:limit] did you mean to NOT change self.contents? Because that's what you got. Whereas: self.contents = self.contents[:limit] would propagate it.

tedunangst 11y ago

Regarding your last point, it's iteratively true. The number of things that go over 120 is much less than the number that go over 100. One always seems to need just a little more space.

pyre 11y ago
I wasn't very clear, but I meant to say that the number of lines that will naturally end up between 80 ~ 100 columns is greater (in practice) than the number of lines that will go over 100 columns. I.e. I think that 100 is a better "sweet spot" than 80 or 120.

rtpg 11y ago

In my case I am a fan of: abort(400, message='All order_items must have a quantity_to_ship.') (not giving the parens an entire line) I'm not a fan of 80 characters, but I am a fan of less than 120 characters, if simply that there is a point where a line won't fit in a terminal thats in half of a "standard laptop" screen (i.e. the limit to where I could put two files next to each other)

s_kilk 11y ago

This is why I dislike Pythons standard of using four spaces for indentation. By the time you've declared a class, a method and a for-loop and a conditional you're already 16 characters down and struggling to fit code into the remaining space.

kevin_thibedeau 11y ago

Just let the line run long. These are only guidelines. There is inherent wiggle room. I break lines that feel too long and casually ignore the 80 column margin. Inserting unnatural breaks is just as bad as excessive line length.

pyre 11y ago
It's different if you're using a tool (e.g. pep8) to ensure coding style. You can tell it to ignore long lines, or you can configure it to have a "better" line length than 80. Telling it to ignore all long lines means absurdly long lines are accepted too, but configuring it for a particular number makes it a hard limit. The best option is to pick a number that is a good trade-off.