Do a database code review

Bug #1409708 reported by PerfectCarl on 2015-01-12
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Footnote
Medium
Unassigned

Bug Description

Review for the Database.vala file in http://bazaar.launchpad.net/~elementary-apps/footnote/footnote/revision/325

1)
Rename Database class (and file) to avoid confusion with SQLHeavy.Database.
-> DatabaseManager if you want to follow Noise convention.

2)
Use a thread safe singleton pattern

        private static GLib.Once<QueueJobManager> instance;

        public static unowned QueueJobManager get_default () {
            return instance.once (() => { return new QueueJobManager (); });
        }

3)
Minor advice: use triple strings for SQL as it makes the reading a lot nicer
Instead of
"CREATE VIRTUAL TABLE notes USING fts3(title TEXT, content BLOB, notebook INTEGER, bookmarked BOOL, date INT64);"
Prefer
"""CREATE VIRTUAL TABLE notes USING fts3(
                title TEXT,
               content BLOB,
               notebook INTEGER,
               bookmarked BOOL,
               date INT64);
"""

4)
Advice: write a method called handle_error that will centralize the handling of all your database related error handling.
It makes things nicer especially if you want to improve on the reportin later on.
(as getting the real SQL message)

PerfectCarl (name-is-carl) wrote :

5)
Use "WHERE id=:id" instead of "WHERE rowid=:id"

6) Add a database version so that your code can migrate your user database schema should it change in the future version
Example:
            db.user_version = 1 ;

7)
Why are you using a full text indexed table (notes) if you never use the MATCH feature?
Why not using a regular table?

Do you plan to add full text search? If that's the case you need the following tables notebook, notes and notes_fts

8)
Note and Notebook are not Gtk widgets they are domain classes (also called model classes).
They shouldn't be in the Widgets namespace or folder.

description: updated
Changed in footnote:
assignee: nobody → PerfectCarl (name-is-carl)
Changed in footnote:
assignee: PerfectCarl (name-is-carl) → Artem Anufrij (artem-anufrij)
status: New → In Progress
Artem Anufrij (artem-anufrij) wrote :

1) done

8) done

Changed in footnote:
importance: Undecided → Medium
Changed in footnote:
assignee: Artem Anufrij (artem-anufrij) → nobody
status: In Progress → Confirmed
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers