On 09/03/2015 08:29 AM, Markus Armbruster wrote: > The QAPI code generators work with a syntax tree (nested dictionaries) > plus a few symbol tables (also dictionaries) on the side. >
> +
> +class QAPISchemaArrayType(QAPISchemaType):
> + def __init__(self, name, info, element_type):
> + QAPISchemaType.__init__(self, name, info)
> + assert isinstance(element_type, str)
> + self.element_type_name = element_type
> + self.element_type = None
Would it make sense to have:
self._element_type_name = element_type
self.element_type = None
> + def check(self, schema):
> + self.element_type = schema.lookup_type(self.element_type_name)
> + assert self.element_type
> +
and the corresponding
self.element_type = schema.lookup_type(self._element_type_name)
to make it obvious that ._element_type_name is for internal use only,
while .element_type is the preferred member for queries/manipulation by
qapi-*.py, which should only be using this object after check() has been
run?
> +class QAPISchemaObjectType(QAPISchemaType):
> + def __init__(self, name, info, base, local_members, variants):
> + QAPISchemaType.__init__(self, name, info)
> + assert base == None or isinstance(base, str)
> + for m in local_members:
> + assert isinstance(m, QAPISchemaObjectTypeMember)
> + assert variants == None \
> + or isinstance(variants, QAPISchemaObjectTypeVariants)
> + self.base_name = base
> + self.base = None
> + self.local_members = local_members
> + self.variants = variants
> + self.members = None
Similarly for self._base_name
> +
> +class QAPISchemaObjectTypeMember(object):
> + def __init__(self, name, typ, optional):
> + assert isinstance(name, str)
> + assert isinstance(typ, str)
> + assert isinstance(optional, bool)
> + self.name = name
> + self.type_name = typ
> + self.type = None
> + self.optional = optional
and for self._type_name
> +class QAPISchemaObjectTypeVariants(object):
> + def __init__(self, tag_name, tag_enum, variants):
> + assert tag_name == None or isinstance(tag_name, str)
> + assert tag_enum == None or isinstance(tag_enum, str)
> + for v in variants:
> + assert isinstance(v, QAPISchemaObjectTypeVariant)
> + self.tag_name = tag_name
> + if tag_name:
> + assert not tag_enum
> + self.tag_member = None
> + else:
> + self.tag_member = QAPISchemaObjectTypeMember('kind', tag_enum,
> + False)
and eventually for self._tag_name (except that we MUST expose
self._tag_name until after we fix the C struct to use 'type' instead of
'kind').
> +class QAPISchemaCommand(QAPISchemaEntity):
> + def __init__(self, name, info, arg_type, ret_type, gen,
> success_response):
> + QAPISchemaEntity.__init__(self, name, info)
> + assert not arg_type or isinstance(arg_type, str)
> + assert not ret_type or isinstance(ret_type, str)
> + self.arg_type_name = arg_type
> + self.arg_type = None
> + self.ret_type_name = ret_type
> + self.ret_type = None
> + self.gen = gen
> + self.success_response = success_response
and for self._art_type_name and self._ret_type_name
> +
> +class QAPISchemaEvent(QAPISchemaEntity):
> + def __init__(self, name, info, arg_type):
> + QAPISchemaEntity.__init__(self, name, info)
> + assert not arg_type or isinstance(arg_type, str)
> + self.arg_type_name = arg_type
> + self.arg_type = None
and for self._arg_type_name
Basically, if I'm understanding the python conventions correctly, naming
an instance member with leading underscore implies that it is for
private use only; and if nothing else, the rename proves whether any of
the other python files are relying on a leaky abstraction.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
