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

Encapsulate check function with content-type #153

Conversation

aswinkarthik
Copy link
Contributor

Minor refactoring to contentType enum. Encapsulated the check function with the type instead of a separate struct.

Copy link
Collaborator

@dineshba dineshba left a comment

Choose a reason for hiding this comment

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

LGTM

@svishwanath-tw
Copy link
Collaborator

svishwanath-tw commented Oct 17, 2019

Create 3 separate types with 3 implementations of check function.
Replace conditionals with polymorphism ...

@svishwanath-tw
Copy link
Collaborator

For example: https://gist.github.com/svishwanath-tw/44f53c6dac8f6f23ae9a951e0e57488e

@svishwanath-tw
Copy link
Collaborator

@dineshba and @aswinkarthik , if there isn't going to be any more action on this PR, i'd like to close it.

@aswinkarthik
Copy link
Contributor Author

@svishwanath-tw I will update it based on your feedback and push.

@svishwanath-tw
Copy link
Collaborator

It has been close to a month now. I'm closing out this PR. Please re-open if you wish to merge.

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.

3 participants