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
