Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[matter_yamltests] Rewrite as_mapping to be more readable #25153

Open
vivien-apple opened this issue Feb 17, 2023 · 1 comment
Open

[matter_yamltests] Rewrite as_mapping to be more readable #25153

vivien-apple opened this issue Feb 17, 2023 · 1 comment
Labels
stale Stale issue or PR

Comments

@vivien-apple
Copy link
Contributor

          ok, but why are we doing that? What is the purpose of having a separate item without `self` just to re-assign to self at the end of the method?

readability-wise this is a very long method, effectively what we seem to want is:

def _get_mappings(self, test, definitions) -> Mappings
   if definitions is None or not definitions.has_cluster_by_name(cluster_name):
      return Mappings(None,None,None)
   if self.is_attribute:
      return self.get_attribute_mappings(test, definitions)
   if self.is_event:
      return self.get_event_mappings(test, definitions)
   
   # assume we are a command (can this be checked?)
   return self.get_command_mappings(test, definitions)

def get_attribute_mappings(self, test, definitions):
    # .....
    return Mappings(arguments=..., response=..., name=...)

#...
def _update_mappings(self, test, definitions):
   mappings = self._get_mappings(test, definitions)
   
   # TODO: if we have a mappings dataclass, may as well not have 3 self variables
   #              and just have self.mappings ....
   self.argument_mapping = mappings.arguments
   self.response_mapping = mappings.response
   self.response_mapping_name = mappings.name

Originally posted by @andy31415 in #25109 (comment)

@stale
Copy link

stale bot commented Oct 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Oct 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issue or PR
Projects
None yet
Development

No branches or pull requests

1 participant