Code review comment for lp:~pitti/platform-api/test-backend

Revision history for this message
Thomas Voß (thomas-voss) wrote :

Looks great, thanks for getting the testing effort started :)

A few minor niggles:

l. 300ff: You can switch "class" to "struct" and get rid of the explicit "public" specifiers. No need to make it a class if default visibility is public.

l. 341ff: Instead of returning a pointer (and managing its lifetime), you could as well return a mutable reference and initialize it as in:

static SensorController& instance()
{
    static SensorController inst_;
    return inst_;
}

With that, the instance is created exactly once on first access, and released after main automatically.

In line 381, you could then make the type->sensor instance mapping non-static, and switch to a shared_ptr for handling the lifetime of the pointer: map<ubuntu_sensor_type, std::shared_ptr<TestSensor>> sensors.

For stylistic karma: The functions handling time could be switched over to use std::chrono (see http://en.cppreference.com/w/cpp/chrono/duration). With that, you don't need comments in the source code specifying the timebase but just return std::chrono::microseconds or std::chrono::nanoseconds. Casting between time bases happens automatically then. With std::chrono, the current time can be sampled with std::chrono::high_resolution_clock::now() or std::chrono::steady_clock::now().

review: Needs Fixing

« Back to merge proposal