Can't do a multiline assignment!

[s0suk3]

Umm, doesn't defining all those members in the class lead to class
variables, not instance variables? I believe the recommended way of
making it clear what instance variables to expect is to initialize them
all in __init__. Currently in your implementation, each instance of
your class is going to share the same variables for all those fields you
defined, which probably isn't what you want.

consider:

class myclass(object):
classvar1=None
classvar2=None

def __init__(self,test):
self.instancevar1=test


6

Also, your idea of checking the length of the headers to reduce the
number of string comparisons is a great case of premature optimization.
First it does not clarify the code, making it harder to follow.
Second, since web servers are I/O bound, it likely does nothing to
improve speed.

So my recommendation is to use a bunch of self.HEADER_NAME=None
declarations in __init__(). This is the expected way of doing it and
all python programmers who are looking at your code will immediately
recognize that they are instance variables.

You didn't really write that at the Python's interpreter, did you?
It's wrong. The way that would really go at the interpreter is like
this:
.... classvar1=None
.... classvar2=None
.... def __init__(self,test):
.... self.instancevar1=test
....6

classvar1 and classvar2 might be class variables, but in they don't
work as they would in C++ or Java (like the ones you declare with the
'static' modified).

 

[Steve Holden]

But. *What's the point* of doing it this way. I see 14 variables
being assigned a value, but I don't see the value, they are getting.
Reading this bit if code provides no useful information unless I'm
willing to scan down the file until I find the end of this mess. And in
that scanning I have to make sure I don't miss the one single line that
does not end in a backslash. (Your ellipsis conveniently left out the
*one* important line needed to understand what this code is doing, but
even if you had included it, I'd have to scan *all* lines to understand
what a single value is being assigned.

There is *no way* you can argue that code is clearer than this:

# General header fields
Cache_Control = None
Connection = None
Date = None
Pragma = None
...

Thank you, you saved me from making that point. It doesn't even seem
like there's a need for each header to reference the same value (though
in this case they will, precisely because there is only one None object).

regards
Steve

 

[s0suk3]

Another thing to consider is that referencing a member of a class or
instance already *is* a dictionary lookup. It's how python works. Thus
dictionaries are optimized to be fast. Since strings are immutable,
python hashes them into a fast lookup pointer. So every time you say
mydict["mykey"], it already knows the lookup pointer (hash) for "mykey"
(the string) and can find the dictionary entry very quickly, ideally in
O(1) time (well maybe log(N)). Thus putting your headers in a
dictionary is actually a really good idea for performance reasons.

I didn't know about that, thanks. So after all the trouble I went
through writing those 200 loc I actually maid it worse...

 

[Michael Torrie]

You didn't really write that at the Python's interpreter, did you?
It's wrong. The way that would really go at the interpreter is like

I did actually run it through the interpreter, but I didn't copy and
past it to the e-mail. Thought that I saw this behavior, but clearly I
must not have.

classvar1 and classvar2 might be class variables, but in they don't
work as they would in C++ or Java (like the ones you declare with the
'static' modified).

Still, it's not pythonic to define instance variables in this way, as
far as I can tell.

 

[Paul Hankin]

class RequestHeadersManager:
...
    def __init__(self, headers, linesep):
        headersDict = parse_headers(headers, linesep)

        for header in headersDict.keys():
...[lots of code]

Your code is pretty much equivalent to this (untested).

class RequestHeadersManager(object):
def __getattr__(self, field):
if not hasattr(self, field):
return None
def __init__(self, headers, linesep):
for header, value in parse_headers(headers,
linesep).iteritems():
header = '_'.join(x.capitalize() for x in
header.split('-'))
setattr(self, header, value)

You may want to add some error checking though!

 

[J. Cliff Dyer]

Thank you, you saved me from making that point. It doesn't even seem
like there's a need for each header to reference the same value (though
in this case they will, precisely because there is only one None object).

regards
Steve

Another possibility is to assign to a dict using a loop, if typing None
over and over again is so onerous.

options = ['cache_control',
'connection',
'date',
'pragma']
params = {}

for option in options:
params[option] = None

And of course you could substitute your choice of appropriate __dict__
for params, if you want to access the options as free-standing objects.

Cheers,
Cliff

 

[Steve Holden]

I do it with all the separate variables mainly for performance. If I
had the headers in a dict, I'd be looking up a string in a list of
strings (the keys of the dict) everytime I check for a header. Not
that that's going to take more that 0.1 seconds, but the program is
still small and simple. As it gets bigger, more features are gonna
slow things down.

So basically you are optimizing for a performance problem you don't
currently have. You want to stop that *straight* away. You are
contorting your logic for absolutely no good reason.

Do what's easiest. Then, when you have that working correctly, speed it
up (if you need to). Start with a dict. How do you know it won't be fast
enough?

regards
Steve

 

[Diez B. Roggisch]

I did actually run it through the interpreter, but I didn't copy and
past it to the e-mail. Thought that I saw this behavior, but clearly I
must not have.


Still, it's not pythonic to define instance variables in this way, as
far as I can tell.

I've seen & used that for quite a while. It is more clear to provide
defaults that way.


Diez

 

[Bruno Desthuilliers]

(e-mail address removed) a écrit :

Another thing to consider is that referencing a member of a class or
instance already *is* a dictionary lookup. It's how python works. Thus
dictionaries are optimized to be fast. Since strings are immutable,
python hashes them into a fast lookup pointer. So every time you say
mydict["mykey"], it already knows the lookup pointer (hash) for "mykey"
(the string) and can find the dictionary entry very quickly, ideally in
O(1) time (well maybe log(N)). Thus putting your headers in a
dictionary is actually a really good idea for performance reasons.

I didn't know about that, thanks. So after all the trouble I went
through writing those 200 loc I actually maid it worse...

That's an understatement.