[Kde-accessibility] [PATCHES] First round of const correctness for the KSpeech interface

Gary Cramblitt garycramblitt at comcast.net
Sat Jan 15 18:29:56 CET 2005


On Saturday 15 January 2005 12:47 am, Matt Rogers wrote:
> Hi,
>
> Attached are two patches that start to establish const correctness for the
> KSpeech interface in kdelibs/interfaces/kspeech. A simple explanation of
> const correctness is that class member functions that modify member
> variables should not be const and those that do not modify member variables
> should be const. The patch for kdelibs also fixes issues with parameter
> default values when passing const references ( 'const QString&' for example
> ) and passing basic types by value that are const ( 'const int' for example
> ). I've checked to make sure that everything compiles, and the patches are
> pretty
> self-explanatory.

Thank you for the patches.  I've committed them.

>
> This is just the first go at this and somebody more familiar with the
> interface and the accompanying code that uses it should continue what i've
> started, since they'll know more about it than I will.

When time permits, I will do a review of the rest of the code for const 
correctness.  I could have used your expertise about 9 months ago. :/

> I would also recommend changing the class name in
> kdelibs/interfaces/kspeech/kspeech.h from 'kspeech' to 'KSpeech'. It would
> then follow the naming convention used throughout KDE. A short changelog of
> my changes is provided below.

That is a historical accident.  When I took over kttsd from pupeno, that was 
the object name and a couple of apps were already using the interface.  I 
think it might be possible to change the name from "kspeech" to "KSpeech" and 
define a new object called "kspeech" that inherits the exact same interface 
as "KSpeech".  The latter would not be documented with doxygen.  This would 
clean up the apidocs and encourage everyone to use KSpeech, but maintain 
backwards compatibility to existing apps.  I wonder if there is a way to do 
this without doubling the marshaling code.


> ---------
>
> Changes:
>
> Remove default values of NULL when passing const references. For one, NULL
> is not the same as QString::null. I just realized that some parameters may
> want a default value of QString::null but nothing barfed on compile, so I
> don't think it's that big of an issue since the implementation of the
> interface seems to define proper default values as well.

I've tested and there don't seem to be any problems so far with your changes.

>
> Remove the const when passing basic types (such as int or bool) by value.
> People will laugh at you if you do it, it's not good C++ AFAIK, and since
> you're passing by value anyways, a copy of the data is made already and you
> don't modify the original.

Thanks.  I'm a newbie at C/C++, so this is educational for me.

>
> All the rest of the changes are to adapt the code so that it's more const
> correct so that it all compiles. There is one FIXME that's been added, but
> i'm sure someone with more knowledge about the code could fix it quicker
> than i can.

That's the problem with const methods, isn't it?  Since TalkerMgr is an object 
for internal use within kttsd, I wonder if it might be simpler to *not* const 
the methods in this case, since signals would be a hassle and complicate 
things, and the goal is more understandable code?

>
> And to think, all of this started because i wanted to eliminate doxygen
> warnings when doing make apidox in kdelibs... :)

Did you succeed in eliminating all the warnings?  I noted them months ago, but 
didn't know how to fix them.

BTW, if you *really* want to help, I need someone to look at the entire kttsd 
project from the standpoint of preparing it for proper translation.  I've 
read, googled, and asked for help on this until I'm bleary-eyed.  For someone 
who fully understands translation, shouldn't require more than an hour or two 
of work.

Thanks again.

-- 
Gary Cramblitt (aka PhantomsDad)
KDE Text-to-Speech Maintainer
http://accessibility.kde.org/developer/kttsd/index.php


More information about the kde-accessibility mailing list