Hi,
the typo resistance is an other good point for using constants.
---
The performance win does not happen when issue a query, it happens when
starting Mixxx.
const QString LIBRARYTABLE_ID = "id";
"id" is in text segment after starting Mixxx. The QString constructor
converts the string to 16 bit unicode and stores it on the heap after
malloc.
const char* = "id";
is simply a pointer to the text segment.
I am also not sure if putting this to trackdao.h will duplicate the sting
in text segment each time trackdao.h is included. So we should make them
static members of TrackDAO to be sure.
Just two cents from an embedded developer ;-)
Kind regards,
Daniel
2013/5/1 RJ Ryan <[email protected]>
> The main reason for me is that they are a protection measure against
> making typos in SQL queries. You'd think a typo would get noticed but not
> all queries fail immediately when they have a mistake in them. Some of them
> just return no results, or return wrong results. For queries that are off
> the beaten path, we end up with issues like
> this<https://bugs.launchpad.net/mixxx/+bug/1175024>(which Steven just fixed).
>
> In fact, I would prefer we totally got rid of writing SQL queries directly
> in code and instead composed them using declarative C++ classes which are
> then compiled down to SQL. Not quite a full-on ORM, just something for
> composing SQL queries (like SQLAlchemy for C++).
>
>
> On Wed, May 1, 2013 at 2:56 PM, Owen Williams <[email protected]> wrote:
>
>> Are the constants really a good idea? What is the purpose behind using
>> them? Ideally the schema shouldn't be changing very often so having
>> them hardcoded seems ok from a maintenance perspective. And from a
>> readability perspective, the constant method looks really ugly to me.
>> I'm just wondering what the other side of the argument is.
>>
>> owen
>>
>> On Wed, 2013-05-01 at 14:42 -0400, RJ Ryan wrote:
>> > On Wed, May 1, 2013 at 2:15 PM, Daniel Schürmann <[email protected]>
>> > wrote:
>> > Hi Steven,
>> >
>> > Yes, we should solve this issue and use the const values in
>> > any case.
>> > This allows to rely on IDE features like "display call tree".
>> >
>> >
>> >
>> > I think we should stick with whatever makes the code easiest to
>> > maintain (which would be using the constants when possible) -- maybe
>> > the constants should be renamed to be a little easier on the eyes? A
>> > lot of the DAO code doesn't use the constants, that's true. We should
>> > fix those queries.
>> >
>> >
>> > But I am not sure if the use of const QString is performance
>> > neutral to const char* and #define because the heavy QSting
>> > constructor is called.
>> >
>> > So IMHO we should change it to const char*.
>> >
>> >
>> > There are a lot of non-performance-neutral choices we make in Mixxx,
>> > but that alone isn't enough to make a decision, we have to take the
>> > context into account. Compared to the QSqlQuery::exec() that comes
>> > immediately after all these queries, some string concatenation isn't
>> > going to break the bank, cycle-wise. Processors are pretty good at
>> > memcpy these days :).
>> >
>> >
>> > Kind regards,
>> >
>> > Daniel
>> >
>> >
>> >
>> >
>> >
>> > Am 01.05.2013 17:32, schrieb Steven Boswell II:
>> >
>> > > Most of the DAO header files in src/library/dao have
>> > > definitions for the names of tables, and the names of
>> > > columns in the tables, but they're used sporadically; quite
>> > > often, references to them are hardcoded.
>> > >
>> > >
>> > > I'm writing a new feature (the aforementioned
>> > > auto-DJ-crates), and the necessary info is stored in a
>> > > temporary table, so I'm writing several SQL queries. I'm
>> > > just wondering if I should hardcode the table/column
>> > > references (which won't automatically get updated if the
>> > > underlying names change), or try to write them using the
>> > > table/column names defined in header files (which makes the
>> > > query harder to read).
>> > >
>> > >
>> > > Here's an example. The comment is the hardcoded,
>> > > human-readable version, and following it is the hard-to-read
>> > > one that'll adapt automatically to changes.
>> > >
>> > >
>> > >
>> > > // INSERT INTO temp_autodj_crates (track_id, craterefs,
>> > > timesplayed, autodjrefs) SELECT crate_tracks.track_id, COUNT
>> > > (*), library.timesplayed, 0 FROM crate_tracks, library WHERE
>> > > crate_tracks.crate_id IN (SELECT id FROM crates WHERE autodj
>> > > = 1) AND crate_tracks.track_id = library.id AND
>> > > library.mixxx_deleted = 0 GROUP BY crate_tracks.track_id,
>> > > library.timesplayed;
>> > > strQuery = QString ("INSERT INTO " AUTODJCRATES_TABLE
>> > > " (" AUTODJCRATESTABLE_TRACKID ", "
>> > > AUTODJCRATESTABLE_CRATEREFS ", "
>> > > AUTODJCRATESTABLE_TIMESPLAYED ", "
>> > > AUTODJCRATESTABLE_AUTODJREFS ")"
>> > > " SELECT " CRATE_TRACKS_TABLE ".%1 , COUNT (*), "
>> > > LIBRARY_TABLE ".%2, 0 FROM " CRATE_TRACKS_TABLE ", "
>> > > LIBRARY_TABLE
>> > > " WHERE " CRATE_TRACKS_TABLE ".%4 IN "
>> > > "(SELECT %5 FROM " CRATE_TABLE " WHERE %6 = 1) AND "
>> > > CRATE_TRACKS_TABLE ".%1 = " LIBRARY_TABLE ".%7 AND "
>> > > LIBRARY_TABLE
>> > > ".%3 == 0 GROUP BY " CRATE_TRACKS_TABLE ".%1, "
>> > > LIBRARY_TABLE ".%2")
>> > > .arg (CRATETRACKSTABLE_TRACKID) // %1
>> > > .arg (LIBRARYTABLE_TIMESPLAYED) // %2
>> > > .arg (LIBRARYTABLE_MIXXXDELETED) // %3
>> > > .arg (CRATETRACKSTABLE_CRATEID) // %4
>> > > .arg (CRATETABLE_ID) // %5
>> > > .arg (CRATETABLE_AUTODJ) // %6
>> > > .arg (LIBRARYTABLE_ID); // %7
>> > >
>> > >
>> > >
>> > > Which should I do?
>> > >
>> > >
>> > > Pretty soon, I'll make a blueprint for the auto-DJ-crates
>> > > feature.
>> > >
>> > >
>> > >
>> > > Steven Boswell
>> > >
>> > >
>> > >
>> > >
>> > >
>> ------------------------------------------------------------------------------
>> > > Introducing AppDynamics Lite, a free troubleshooting tool for
>> Java/.NET
>> > > Get 100% visibility into your production application - at no
>> cost.
>> > > Code-level diagnostics for performance bottlenecks with <2%
>> overhead
>> > > Download for free and get started troubleshooting in minutes.
>> > > http://p.sf.net/sfu/appdyn_d2d_ap1
>> > >
>> > >
>> > > _______________________________________________
>> > > Get Mixxx, the #1 Free MP3 DJ Mixing software Today
>> > > http://mixxx.org
>> > >
>> > >
>> > > Mixxx-devel mailing list
>> > > [email protected]
>> > > https://lists.sourceforge.net/lists/listinfo/mixxx-devel
>> >
>> >
>> >
>> >
>> ------------------------------------------------------------------------------
>> > Introducing AppDynamics Lite, a free troubleshooting tool for
>> > Java/.NET
>> > Get 100% visibility into your production application - at no
>> > cost.
>> > Code-level diagnostics for performance bottlenecks with <2%
>> > overhead
>> > Download for free and get started troubleshooting in minutes.
>> > http://p.sf.net/sfu/appdyn_d2d_ap1
>> > _______________________________________________
>> > Get Mixxx, the #1 Free MP3 DJ Mixing software Today
>> > http://mixxx.org
>> >
>> >
>> > Mixxx-devel mailing list
>> > [email protected]
>> > https://lists.sourceforge.net/lists/listinfo/mixxx-devel
>> >
>> >
>> >
>> ------------------------------------------------------------------------------
>> > Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
>> > Get 100% visibility into your production application - at no cost.
>> > Code-level diagnostics for performance bottlenecks with <2% overhead
>> > Download for free and get started troubleshooting in minutes.
>> > http://p.sf.net/sfu/appdyn_d2d_ap1
>> > _______________________________________________ Get Mixxx, the #1 Free
>> MP3 DJ Mixing software Today http://mixxx.org Mixxx-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/mixxx-devel
>>
>>
>>
>
------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with <2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Get Mixxx, the #1 Free MP3 DJ Mixing software Today
http://mixxx.org
Mixxx-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mixxx-devel
