Skip to content
This repository has been archived by the owner on Aug 26, 2024. It is now read-only.

Optimized fuzz.py #136

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Optimized fuzz.py #136

wants to merge 4 commits into from

Conversation

rahul-nath
Copy link

Simplified logic using De Morgan's Laws. Combined some lines to reduce string concatenation costs and make coding decisions more uniform. Hope they help.

@josegonzalez
Copy link
Contributor

Sorry for not looking at this sooner. Is this a speed optimization, and if so, do you have benchmarks?

Comment on lines +160 to +163
# sort, join, and strip
sorted_sect = (" ".join(sorted(intersection))).strip()
combined_1to2 = (" ".join([sorted_sect, " ".join(sorted(diff1to2))])).strip()
combined_2to1 = " ".join([sorted_sect, " ".join(sorted(diff2to1))]).strip()

Choose a reason for hiding this comment

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

This is pretty much the same, but not completely. In this changed code whitespace from sorted_sect is stripped before the strings are combined, while it was done afterwards in the original code. This does not matter in this case, since the original strip for sorted_sect was not required, since " ".join(sorted(intersection)) can never have whitespaces in begin or end. From a performance standpoint this does not really matter, but is probably slightly slower, since it creates some intermediate lists and calls the join function more often.

if not utils.validate_string(p1):
return 0
if not utils.validate_string(p2):
if not (utils.validate_string(p1) and utils.validate_string(p2)):

Choose a reason for hiding this comment

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

Again no performance implication. Simply a preference, whether these two if statements should be joined

@maxbachmann
Copy link

@josegonzalez It's a line count optimisation. However in my opinion at least the string joining hurts readability.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants