knadh / localStorageDB

A simple database layer for localStorage and sessionStorage for creating structured data in the form of databases and tables
http://nadh.in/code/localstoragedb
814 stars 128 forks source link

Added ability to get a sorted (query) result #34

Closed orangecoding closed 10 years ago

orangecoding commented 10 years ago

Hi,

I've added a new function named "sortedQuery". It works like query but it sorts the query results. If you query data from within the localStorage, you'll get the ID's of the rows. Afterwards you'll select the data with the ids. The ID's are within an object. Now if I've changed this object into an array. An array has a native sort function which I use to sort the ids. After this sorting, I'll use your query method as it's intend to use. I've added a new method (and not changed the actual qury method) to not break your API. Anyway, It would be much (muuuuch) better to chnage the method signature of query to query(table_name, obj). The Obj could contain things like limit, start or event sorting information. Also your library is now AMD compatible.

What I've changed:

knadh commented 10 years ago

Whew, that's loads of big commits. I'll need some time to review and understand.

Something that caught my attention; why have you changed the signature+logic of getIDs(). In the original version, it takes the table name, but in your version, it takes the actual table data. Why pass around the table data when all the methods in the class are written to access data from the single instance assigned within the class?

Thanks!

orangecoding commented 10 years ago

Hey, you're right, I've used the internal code formatter from webstorm which seems to be a bad idea. this is why the first commit is red all over.. I've changed getIDs() and the other 2 query functions to directly get the table data because 1) I wanted them all to have nearly the same method signature 2) queryByValues() and queryByFunction() will also be called from sortedQuery(). But in sortedQuery() I'll created a sorted array of id's which i pass to the query function. In this case I don't need the table name but the sorted data.

knadh commented 10 years ago

Can you withdraw this pull request and submit a new one with minimal changes? The current commits bring in way too many non-important changes, including style inconsistencies.

With regards to getIDs(), the change doesn't seem to bring any additional advantages. Moreover, it becomes inconsistent with all other functions that receive the table_name and access the data from a single instance, rather than receiving all the table data in an argument.

Thanks

knadh commented 10 years ago

:-)

orangecoding commented 10 years ago

Please close this issue, I've added a new one: https://github.com/knadh/localStorageDB/pull/35

knadh commented 10 years ago

Closing in favour of https://github.com/knadh/localStorageDB/pull/35