chenyanming / calibredb.el

Emacs calibre client - An Ebook Management Solution in Emacs.
GNU General Public License v3.0
318 stars 16 forks source link

Simplifying functions, fixing typos and add error handling in calibredb-query #46

Closed c1-g closed 2 years ago

c1-g commented 3 years ago

The pull request from #45 is now based on the develop branch.

Also, you might have noticed that the number of commits in this pull request is less than the last one, this is because I squashed minor commits together so that they are easier to read through.

And one last thing, in #45 I'd exclaimed that alists are not suitable for this package and that we should use plists or hash tables instead to fix speed issue, I take back my word. What I called "speed issue" is too broad to really carry any meaningful message, the "speed issue" of this package is actually from the time querying the sqlite database of calibre when there are a large amount of books, there is not much we can do about that, either some sql wizards can optimize the code in calibredb-query-string to cut down some time or somebody comes up with a new shell command to query the database in calibredb-query altogether. True, storing the queried entries in hash tables maybe slightly faster for live filter but, it doesn't help the load time or anything and the time refactoring the code and testing is too much for what is essentially a non-issue. (Or maybe not? I don't know, I don't have that many books in my calibre (< 100) do you experience slowness with live filter with many books?)

TLDR; Hash tables are not worth it.