Skip to content
This repository has been archived by the owner on May 8, 2019. It is now read-only.

Refactored and fixed the add_slashes function #62

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

Refactored and fixed the add_slashes function #62

wants to merge 2 commits into from

Conversation

mekhami
Copy link

@mekhami mekhami commented May 1, 2015

Two things I found wrong with this function. One, there's no reason to define it within the top level function. Something like this is both reusable, doesn't play with weird namespaces, so it should be made available at the top level.

Second, it didn't actually do anything. Period. Strings are immutable, so it does all the replaces you want but doesn't actually "save" the results.

@ghost
Copy link
Author

ghost commented May 1, 2015

In general it's probably just pointless to even have the add_slashes function when you could just use re.escape. But that's not something I'm gonna decide for you :)

@kaedroho
Copy link
Contributor

Thanks!

Yep, add_slashes was added just reduce duplication. It looks like that could be replaced with re.escape though!

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.

2 participants