[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