Skip to content

Refactor out unique assertion #225

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.

Sign up for GitHub

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

Closed
mattsaxon wants to merge 1 commit into from
Closed

Refactor out unique assertion #225

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 6 additions & 24 deletions zeroconf/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,21 +882,15 @@ class State(enum.Enum):
init = 0
finished = 1

@staticmethod
def is_type_unique(type_: int) -> bool:
return type_ == _TYPE_TXT or type_ == _TYPE_SRV or type_ == _TYPE_A or type_ == _TYPE_AAAA

@jstasiak jstasiak Feb 3, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just type_ in {_TYPE_TXT, _TYPE_SRV, _TYPE_A, _TYPE_AAAA}?


def add_question(self, record: DNSQuestion) -> None:
"""Adds a question"""
self.questions.append(record)

def add_answer(self, inp: DNSIncoming, record: DNSRecord) -> None:

"""Only support for unique answers"""
if (
record.type == _TYPE_TXT
or record.type == _TYPE_SRV
or record.type == _TYPE_A
or record.type == _TYPE_AAAA
):
assert record.unique

Comment thread
jstasiak marked this conversation as resolved.
"""Adds an answer"""
if not record.suppressed_by(inp):
self.add_answer_at_time(record, 0)
Expand All @@ -905,13 +899,7 @@ def add_answer_at_time(self, record: Optional[DNSRecord], now: Union[float, int]
"""Adds an answer if it does not expire by a certain time"""
if record is not None:

"""Only support for unique answers"""
if (
record.type == _TYPE_TXT
or record.type == _TYPE_SRV
or record.type == _TYPE_A
or record.type == _TYPE_AAAA
):
if self.is_type_unique(record.type):
assert record.unique

@jstasiak jstasiak Feb 3, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could put the assert in the function/method too and call it assert_unique_used_correctly or something I think.

@mattsaxon mattsaxon Feb 3, 2020

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered it, but thought it was more readable as I have it. I pondered it for a while and decided the readability was better than saving the one line. I'd appreciate your view to be honest and could have gone either way.

@jstasiak jstasiak Feb 3, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair, looks good either way.


if now == 0 or not record.is_expired(now):
Expand Down Expand Up @@ -957,13 +945,7 @@ def add_additional_answer(self, record: DNSRecord) -> None:
o All address records (type "A" and "AAAA") named in the SRV rdata.

"""
"""Only support for unique answers"""
if (
record.type == _TYPE_TXT
or record.type == _TYPE_SRV
or record.type == _TYPE_A
or record.type == _TYPE_AAAA
):
if self.is_type_unique(record.type):
assert record.unique

self.additionals.append(record)
Expand Down