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

Make Filter methods static to avoid the need for instantiation #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

benbarbersmith
Copy link
Contributor

Hi Tyler,

This PR is a little opinionated, and is a breaking change, so I'll understand if you'd rather not merge it. But I couldn't resist sending it your way.

The current library requires users to create a Filter object before they can use the isProfane or clean methods. But Filter has are no class variables, and there is no work performed by the Filter constructor. So there's no need or benefit from creating an instance of Filter before accessing those methods.

If we make the isProfane and clean methods static, users can call isProfane and clean without first instantiating Filter. I don't see any downside to this. Even if you plan to add configuration options in the future, you could handle those options much like Logger and Logger.root in package:log/log.dart — no need for instantiation.

Making these methods static is a breaking change, and would probably warrant a bump to version 0.3.0.

If existing users of the library upgrade, they would need to make some very minor changes. For example, they would replace this:

final filter = Filter();
filter.isProfane('a string');
final cleanString = filter.clean('a string');

with this:

Filter.isProfane('a string');
final cleanString = Filter.clean('a string');

Like I said, feel free to ignore this PR if you'd rather not make breaking changes. But I think this improves the ergonomics of the library hugely, so I am in favour. 👍

Cheers,
Ben

This is a breaking change and would warrant a bump to version 0.3.0. But I suspect many would prefer to avoid initializing Filter every time for no benefit, and it may also come with a minor performance increase (as now everything is effectively constant, including the methods).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant