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

A more complete and understandable implementation of the search function #28

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

Conversation

baldassarreFe
Copy link

I wrote a more complete and understandable implementation of the search function. It deals with missing parameters as wildcards and can manage both single keywords and list of keywords. Can it be pulled into the main repository?

@filipkis
Copy link
Contributor

Thank you for the effort and improving our examples. This kind of efforts are appreciated and welcomed.

Before this can be pulled in I would, however, request the following:

  • rewriting the advanced JS features not yet widely supported (e.g. arrow functions)
  • merging the code with current getAllDishes function in order to not have two functions that do the same thing. The old way of searching (type, filter) should also work (for backward compatibility). I guess only flipping the order in your method would be needed, but please make sure it logically works the same way.

I admit that getAllDishes is unfortunate name as it's not alway getting all the dishes. Therefore, there could be an alias method called search, and we could deprecate the getAllDishes slowly. However, first we need to have them both working the same.

With reference to: kth-csc-iprog#28 (comment)
Now the search function, renamed getAllDishes, mimics the original getAllDishes, with the difference that calling without parameters actually returns all the dishes as the name suggests.
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.

2 participants