ScreenController able to manage different type of Planar TV #229
Conversation
| @@ -0,0 +1,96 @@ | |||
| /*********************************************************************/ | |||
| /* Copyright (c) 2017, EPFL/Blue Brain Project */ | |||
| std::vector<std::unique_ptr<ScreenController>> controllers; | ||
|
|
||
| controllers.push_back(std::unique_ptr<ScreenController>(mockController1)); | ||
| controllers.push_back(std::unique_ptr<ScreenController>(mockController2)); |
There was a problem hiding this comment.
controllers.emplace_back(new MockScreenController(ScreenState::ON));
Naked "new" as you did above are forbidden... :-)
| controllers.push_back(std::unique_ptr<ScreenController>(mockController1)); | ||
| controllers.push_back(std::unique_ptr<ScreenController>(mockController2)); | ||
|
|
||
| auto multiController = new MultiScreenController(std::move(controllers)); |
There was a problem hiding this comment.
...and here you have a good reason why: this multiController is never deleted! You could have used make_unique() instead, but in fact there is no reason to allocate one on the heap. Just create one on the stack.
|
|
||
| multiController->powerOn(); | ||
| BOOST_CHECK_EQUAL(multiController->getState(), ScreenState::ON); | ||
| } |
There was a problem hiding this comment.
There should be a check that when any of checkPowerState / powerOff / powerOn are called, the MultiController is indeed calling the same functions on all the MockControllers that were passed to it.
|
|
||
| } | ||
|
|
||
| BOOST_AUTO_TEST_CASE(testSignals) |
There was a problem hiding this comment.
Which signals? I am not seeing the powerStateChanged signal being tested and that's the only one that this class has..?
| auto serialports = ports.split(';'); | ||
| if (serialports.length() == 0) | ||
| const auto connections = processInputString(ports); | ||
| if (connections.size() == 0) |
| QMap<QString, PlanarController::Type> map; | ||
|
|
||
| const auto connections = ports.split(';'); | ||
| if (connections.length() == 0) |
| const auto connections = ports.split(';'); | ||
| if (connections.length() == 0) | ||
| return map; | ||
| for (auto connection : connections) |
|
|
||
| if (connection.contains("#")) | ||
| { | ||
| auto serialEntity = connection.split("#"); |
| QMap<QString, PlanarController::Type> | ||
| ScreenControllerFactory::processInputString(const QString& ports) | ||
| { | ||
| QMap<QString, PlanarController::Type> map; |
There was a problem hiding this comment.
use a typedef for QMap<QString, PlanarController::Type>? It's quite long and repeated a lot of times.
| if (connection.isEmpty()) | ||
| continue; | ||
|
|
||
| if (!connection.endsWith("#") && connection.contains("#")) |
There was a problem hiding this comment.
to be reviewed, at least inverse the two conditions for readability. If someone inserts abc#def# this jumps to the second case, which is not the intention. I would still prefer a single code path with a split('#') at the beginning.
| std::vector<std::unique_ptr<PlanarController>> controllers; | ||
| for (const auto serialport : serialports) | ||
| std::vector<std::unique_ptr<ScreenController>> controllers; | ||
| for (const auto& kv : connections.toStdMap()) |
ppodhajski
force-pushed
the
master
branch
16 times, most recently
from
f31c2cd
to
c3432fc
Compare
ppodhajski
force-pushed
the
master
branch
4 times, most recently
from
3d2f209
to
2010954
Compare
No description provided.