[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