HackMyChurch / aelf-dailyreadings

AELF daily readings is the easiest and most discrete daily reading application on the play store. https://play.google.com/store/apps/details?id=co.epitre.aelf_lectures
MIT License
47 stars 14 forks source link

Sort chapters during the extraction + add verse ids (integers) #45

Closed 1sixunhuit closed 1 year ago

1sixunhuit commented 1 year ago

J'ai effectué ces modifications pour faciliter la création d'une API qui ira lire cette base de donnée afin d'intégrer la Bible à un bot Discord


This change is Reviewable

yadutaf commented 1 year ago

Hello ! Thanks for your proposition ! I'm not sure to understand the rational behind this change. It is already possible to get verses / chapters in order with proper ORDER BY clauses. Actually, this is precisely what the AELF app is doing :smile:

Additionally, from a database design point of view, there should not be an implicit information in a table. Typically, relying on row order is generally a bad idea, especially in contexts where a row may change. Of course, this is not a problem here since the database is generated once and then only queries. I however really prefer to stick with best practices !

If your application's goal can not be achieved without changing the databse schema, could you make sure to re-generate the database and make sure the application still works with the schema change? That said, again, I believe these changes are not necessary to get the verses and chapters in order :smile:

1sixunhuit commented 1 year ago

Thanks for your reply ! Use ORDER BY to sort the database was not working well in my case, but maybe It's because I'm not used to write SQL requests (I only use it few years ago during 1 month at school and only on paper, never in front of a screen...). I tried with something like this :

SELECT verse, text
FROM verses WHERE book LIKE 'Jn' AND verse >= 20 AND verse <=40 AND chapter = 10 ORDER BY  verse

And with this, I get verse 3 between verses 29 and 30... (and also, I don't understand why I have to use "LIKE" and not "=" for the book, but it's working, so...).

I know the app is working well without this, but I didn't understood how you manage with it, and that's why I suggested it :sweat_smile:

I will try to run the application with my modifications tomorrow evening and I'll give you news (I have to often the bad practice to push and try after :joy: ) ;)

yadutaf commented 1 year ago

To get the chapters in order, the relevant code in the app is:

Cursor cursor = db.query(
                true,
                "verses",
                new String[]{"chapter", "chapter_title", "book_title"},
                "book = ?",
                new String[]{bookRef},
                null,
                null,
                "CAST(chapter AS INTEGER)",
                null
        );

The trick is to "CAST" the chapter (or verse) as integer. If I remember correctly, I had to store the chapters as strings rather than integers because some chapters have "A" and "B" suffixes, like some psalms. But this is clearly not a great design. I should probably have split the integer part from the suffix part...

For the verses, the code is:

Cursor cursor = db.query(
                "verses",
                new String[]{"verse", "text"},
                "book = ? AND chapter = ?",
                new String[]{bookRef, chapterRef},
                null,
                null,
                "rowid",
                null
        );

And this clearly relies on insertion order. No comment... Poor design... (And yes, I'm the author... 😜)