search

dhavalpzala / multi-hashmap

MIT License
3 stars 2 forks source link

rename getRecords() to getAll() #1

Closed praveenpuglia closed 8 years ago

praveenpuglia commented 8 years ago

Yeah! like findAll(). Think that goes with the naming convention?

dhavalpzala commented 8 years ago

nice point :+1:

praveenpuglia commented 8 years ago

Now that I have sent the pull request, I have broader question for APIs, deriving from #2 .

Aren't findAll() and getAll() confusing together? I think the API can actually be more simplified.

insert(key, value);

find(key); // Find the first value associated with the key. We basically don't care about the value here.
find(key, value); // Find the first value that matches the value param for the associated key.

findAll(key); // Find all values that are associated with that key.
findAll(key, value) // Find all the values that match the value param for the associated key.
findAll() // Finds all records

Should I create another issue for this one for modify #2 ?

@dhavalpzala - any thoughts?