[Kde-finance-apps] Review of alkquotes

[Alvaro Soliverez]

Hello Brian,
here is a first review of the code:

- For strings that will be displayed to the user, please use i18n, so
the strings can be translated
- Create a folder to drop all CMake modules there
- Add unit tests, at least for all public methods in the backend classes
- When initializing a class, QString should be initialized on the same
call and not on the body.

CompanyInfoPlugin::CompanyInfoPlugin()
{
  m_pluginName = "CompanyInfoPlugin";

should be

CompanyInfoPlugin::CompanyInfoPlugin():
 m_pluginName("CompanyInfoPlugin")
{

That is more efficient, because it saves a QString instance.

- Avoid using "select *" on queries. That's prone to errors in the
future and less efficient as tables start growing
- You should use queries with bind values as opposed to create the
whole query string each time.

query.exec( "SELECT * FROM symbolinfo WHERE symbol='" + symbolCAPS + '\'' );

should be similar to this example in
http://doc.trolltech.com/4.6/qsqlquery.html#details

QSqlQuery query;
     query.prepare("INSERT INTO person (id, forename, surname) "
                   "VALUES (:id, :forename, :surname)");
     query.bindValue(0, 1001);
     query.bindValue(1, "Bart");
     query.bindValue(2, "Simpson");
     query.exec();

That's much more efficient, and much more secure if the queries are
open to user intervention.

- Get rid of QDebug statements on the commited code. That suggests
that unit tests are extremely necessary. As a rule, don't commit code
with QDebug (unless it is commented out, of course)

- I see that a number of classes have an interesting amount of
parameters which all seem to be related.
For example:
searchResults( const QString &searchTerm,
			const QStringList &symbols,
			const QStringList &names,
			const QStringList &exchanges,
			const QStringList &types );

You should create a class to contain these values and just pass an
instance of it. The patter is called Data Transfer Object (DTO)

- I've noticed you added some krazy excludes. Is there a reason for that?
  Krazy excludes should be added only as last-instance, when it is
certain there is a false positive.

*  Copyright (c) 2010 Brian Cappello 		//krazy:exclude=copyright
  *  briancappello [at] gmail [dot] com

In this case, Krazy could not find the email, probably. Using [at] and
[dot] won't stop the email harvesters, so use the proper form and
relay on the gmail spam filter.


And, to emphasize, test, test, and test. :)

I still have to review the design. I'll get to it during the next few days.

Regards,
Alvaro