[PULL REQUEST] Subsurface branch - QtBluetooth serial implementation
Thiago Macieira
thiago at macieira.org
Mon Jul 6 11:16:47 PDT 2015
On Monday 06 July 2015 17:42:00 Claudiu Olteanu wrote:
[mostly cut]
Hello Claudiiu
The patches look good. They do what we had discussed previously.
My comments are only nitpicks and suggestions for coding style in the future.
There's no need to change anything now.
> + bool on = !(mode == QBluetoothLocalDevice::HostPoweredOff);
That's slightly weird :-)
I can see how this can grow from having a !(x || y) and then you remove part
of the conditional.
> + QString deviceLable = QString("%1
> (%2)").arg(remoteDeviceInfo.name()).arg(remoteDeviceInfo.address().toString
> ());
Hint: QString has a "multiarg" arg that works on QStrings and is slightly
faster. If you remember, please use it next time.
QString("%1 (%2)").arg(emoteDeviceInfo.name(),
remoteDeviceInfo.address().toString());
The variable name is misspelt.
> +
> + // Check if the remote device is already in the list
> + if (itemsWithSameSignature.empty()) {
Hint: please use the Qt API for the containers. I had to look up to see if the
empty() function was a getter of the emptiness state or whether it emptied the
container.
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
PGP/GPG: 0x6EF45358; fingerprint:
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
More information about the subsurface
mailing list