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.
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.
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.
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).
> Interesting you find aliasing in variables 'dirty'.
I don't find aliasing by itself to be dirty. It is just in this example, it would not be adding semantic meaning (as in the "named boolean expressions" example) and it would be used in only a single place (immediately on the next line) so I find it to be the least justified use of aliasing.
Why use them then if you have autocomplete? Since character variable names are useful as very short conventions, I'd never want to see them used generally. Someone working in this code would have to know the meaning immediately from the name, as for any variable. In my application area x,y,w,h,r (radius) and c (context - which is domain specific, but is present in about 50% of LOC in my current code-base) are all as ubiquitous as i and j (I'd probably not be comfortable with k, too deep nesting). i and j are much rarer in python, either using sequences of underscore for 'don't care'.
On the second point, there is reason, as I pointed out. It makes code shorter, clearer, and in some cases more performant (when contents is a property, for example). Don't get me wrong, I was following up on the specific point, I wasn't trying to recommend this style as good general practice (alias all your self variables at the top of a method - erm, no).
The lack of 'const'ing in python is a risk, yes. We use immutable data structures whenever possible, including immutable classes, because we try to ensure as much code is written without side effects on data. I'd want to not alias variables that have side effects applied to them, yes.
But you're right that it makes it easier for a sloppy edit to mess things up. In most cases the 'sloppy' would be adding the side effects at all, however, which either version has.
particularly as the number of uses of x and y increase beyond two in a functon.
Ultimately code-quality and maintainability can't be mandated by a PEP or any universal rule. But like any field, you have to understand the purpose of the rule, and weigh up its risks and benefits before you break it.
Well, for mathematics and such cases (physics etc), yes, I agree that x vs self.position.x etc is handier.
But I wouldn't ever do it for the "more performant" part, unless I have profiled the code and it indeed makes a non trivial difference (I know aliasing to a local makes a difference it itself - but I mean it should be significant when the whole program is taken into account, even if it was 1000x faster to access the aliased local, it wouldn't make sense to optimize a part of the program with it, if said part is only responsible for 1% of the total running time).
For the general case, I suggested i, j, k only because they are well known conventions, so their meaning will be instantly understood (and of course, if one can avoid such indexes altogether, e.g. with a "for item in" loop, one should).
I think 3 is bad because it is more complex than the natural code. It's not incomprehensible, of course, but clearly more complex.
I like introducing named variables for readability a lot. But this doesn't help readability. It just adds complexity for the sake of fitting into 80 characters.
> But this doesn't help readability. It just adds complexity for the sake of fitting into 80 characters.
But 'for the sake of' implies that it has no other purpose. But the purpose is readability. It will help readability for the vast population of programmers who have 80 col editors or shells, and who code according to PEP8, won't it? Isn't that the point? Of course you can say 'It would be more readable if you guessed my window width preference correctly'. But what should I guess? If I look at your code and see a 90 col line somewhere, how do I know you won't drop a 100 col line in and hurt the readability again? Do I have to read your README to find how big to make my windows? Or do I just take the readability hit for the sake of your preference?
1. I don't remember when I last ran into anyone who voluntarily limited their code to 80 characters. I doubt it was this century. I worked for a while at a Major Company that had an 80 character coding standard. It was universally despised on my team, where everyone had 2x27" monitors.
Clearly you work in a different environment with different standards. I think more or less formal coding standards are important for any dev team. But they can be very different in different teams. Don't assume what you have grown used to and comfortable with is some universal truth that must fit everyone for all times.
> But what should I guess? If I look at your code and see a 90 col line somewhere, how do I know you won't drop a 100 col line in and hurt the readability again?
If we work on the same team, that is a discussion we'd need to have. Until then, let's continue being happy in our respective circumstances, OK?
2. By "Readability" I mostly mean "cognitive load" for the human reader of the code. What to fit in a line is mostly decided by what's an easily digestible chunk. If you can fit "one thought" into one line, that's ideal. Line length is a factor, but just one of many, not one that overrides every other.
So, come to think of it, I actually would break out a variable solely for line length reasons. Absolutely not for the sake of 1 character, but probably for 20.
It will help readability for the vast population of programmers who have 80 col editors or shells
Isn’t that the premise that is being challenged here, though? There’s plenty of variety in which editors and shells experienced programmers prefer to use, but do any of those tools really still have a hard 80 column limit in 2015?
I would guess most developers now use screens that can show at least two (and often three or even four) source files side by side and still cope with longer lines than that, and I would guess most could switch their editor or shell to cope with those wider lines in a fraction of a second with a single keyboard shortcut.
I can see cases where line length could still be a limiting factor, but I’m not sure giving up general readability improvements from allowing somewhat longer lines so it’s easier to do a three-way merge on a laptop with a smaller screen is a good trade.
Nobody is suggesting that people Can't view more than 80 cols. But many people have their machines and environments set up for 80 cols. They have their windows laid out based on the code they're writing, and then they load a new file and have to drag and resize their environment to accommodate you.
If there's a way to change window size and position from 3 columns of 80 + a project browser into 120 col with a single keypress, then I'd like to know. It isn't a major problem, but it is an unnecessary problem.
Nobody is forced to use long lines either, and if you want to lay out your environment to support 120 cols and you get my 80 col source code it will work without even a keypress or a second thought. You won't even notice it.
If there's a way to change window size and position from 3 columns of 80 + a project browser into 120 col with a single keypress, then I'd like to know.
In case it’s of any interest:
I use Sublime Text as my main editor. One of its nice features is that you can switch how the window is divided up with a single keyboard shortcut or menu command (View -> Layout). You can also easily define custom layouts in addition to the standard 2/3/4 columns, 2/3 rows, and 2x2 grid. I usually run with the editor maximised on my largest screen and 2–4 different panes open. The one feature it’s missing in this area that I’d really like to have is similarly clean support for multiple windows and therefore multiple screens. I’m not an Emacs expert, but colleagues who use it regularly do seem to have similar features available.
I also use a tool called WinSplit Revolution, which provides a somewhat similar feature for general Windows applications, letting you define some preset regions of your screen and then snap the current window to any of them with a quick keyboard shortcut. It can also shift windows between multiple monitors if you have them, again with a single quick shortcut. I can therefore instantly resize windows for things like shells, history/diff tools, and so on if I need them to be a bit wider for whatever files I’m currently working with.
That is semantically different. It always calls abort(). In this case abort() raises an exception that returns a HTTP 400. I'm not going to abort if everything seems fine. :P
If you wanted to do this pattern, it would look more like this:
message = None
if not_shipment_qty:
message = "Shipment is not fine"
elif shipment_qty < 0:
message = "No quantity"
if message:
abort(400, message=message)
(Note: In this case the keyword argument form of message is required, because all keyword args get packaged up into a JSON body of the response.)
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.
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.
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)
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.
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.
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.
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.