erinhengel / Textatistic

Calculate readability scores
http://www.erinhengel.com/software/textatistic/
Apache License 2.0
40 stars 9 forks source link

don't instantiate argument defaults in the function declaration #2

Closed rascalking closed 2 years ago

rascalking commented 5 years ago

doing so means they get instantiated on import, and potentially reused across multiple calls to a function.

also, if you're running as a user that doesn't have a home directory, that means that the Hyphenator('en_US') call fails with a PermissionError before the caller has a chance to pass their own in.

kfogel commented 2 years ago

This PR looks good to me, except that it might be slightly better to use if foo is None: instead of if not foo: for all the conditionals (that'd be a trivial change to apply consistently throughout the PR). Thanks, @rascalking.

@erinhengel, if you're no longer maintaining this repository, then I'll fork a copy so I can apply this PR. But I wanted to check first, because if you are still maintaining Textatistic, then I'd much rather stay on your upstream copy and not run my own fork.

erinhengel commented 2 years ago

Hey @kfogel. No I'm no longer maintaining this repository so please go ahead and fork it.

kfogel commented 2 years ago

Will do, and thanks for the prompt & clear reply, @erinhengel.

kfogel commented 2 years ago

Okay, https://github.com/kfogel/Textatistic has now merged this PR on its master branch, with my is None change added on top (for all such conditionals, not just the ones in this PR).