Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm writing a struct class in Python and was wondering if this were a good way to write it:

class Struct:
    def __init__(self, **kwargs):
        for key, value in kwargs.items():
            if isinstance(getattr(self, key), str):
                setattr(self, key, value)

    def create(*args):
        newStruct = type("Struct", (Struct,), {"__init__": Struct.__init__})
        for arg in args:
            setattr(newStruct, arg, str())
        return newStruct

and to initialize the object:

myStruct = Struct.create('x', 'y')
variable = myStruct(x=2, y=4)

Also, is it actually worth using?

share|improve this question
1  
FWIW: github.com/lihaoyi/macropy uses a "case" macro which is similar to what you have. – Dair Feb 13 at 8:50
up vote 4 down vote accepted

This looks like a mutable version of namedtuple, with some oddities.

Following from your example (myStruct = Struct.create('x', 'y'); variable = myStruct(x=2, y=4)), I would expect variable.z to fail — and indeed it does raise an AttributeError: 'Struct' object has no attribute 'z'. I would also like to see variable.z = 5 fail with the same error, but it doesn't. So, if it doesn't enforce what members can be set, what's the point of this object? Why wouldn't I just use a regular dict?

The __init__ function looks like a constructor, such that I would be tempted to write myStruct = Struct('x', 'y') instead of myStruct = Struct.create('x', 'y')) — but it actually doesn't work that way. Also, I'd expect the constructor to work like namedtuple, accepting a typename, followed by field_names as either a list or a space-delimited string.

Defaulting values to an empty string is weird; I'd expect the default values to be None. You can initialize the dictionary using dict.fromkeys().

It would be nice to have __repr__() overridden, to make it easy to inspect the objects' contents for debugging.

Suggested implementation

def Struct(typename, field_names):
    class StructType:
        def __init__(self, **kwargs):
            for key, value in kwargs.items():
                setattr(self, key, value)

        def __setattr__(self, key, value):
            if hasattr(self, key):
                super().__setattr__(key, value)
            else:
                getattr(self, key) # Trigger AttributeError

        def __repr__(self):
            return repr(self.__dict__)

    return type(
        typename,
        (StructType,),
        dict.fromkeys(field_names.split() if isinstance(field_names, str)
                      else field_names)
    )

Sample run:

>>> myStruct = Struct('myStruct', 'x y')
>>> variable = myStruct(x=2)
>>> variable
{'x': 2}
>>> variable.y
>>> variable.y = 4
>>> variable.y
4
>>> variable.z = 5
Traceback (most recent call last):
  …
AttributeError: 'myStruct' object has no attribute 'z'
>>> variable.z
Traceback (most recent call last):
  …
AttributeError: 'myStruct' object has no attribute 'z'
share|improve this answer
    
Thanks! I had originally thought that z=5 would not work. – ChaiNunes Feb 13 at 18:35
    
getattr(self, key) seems needlessly cryptic. How about raise AttributeError(key) instead. – ppperry Jun 2 at 2:01

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.