Pelagicore / gdbus-codegen-glibmm

Code generator for C++ D-Bus stubs and proxies using Giomm/Glibmm
GNU Lesser General Public License v2.1
23 stars 25 forks source link

Property names unnecessarily extracted when getting property values in proxy. #84

Closed martin-ejdestig closed 5 years ago

martin-ejdestig commented 5 years ago

Gio::DBus::Proxy::get_cached_property() returns an empty (bool operator returns false) variant if property is not cached. That is, this:

{{ prop.cpptype_out }} {{ class_name_with_namespace }}::{{ prop.name }}_get()
{
    std::vector<Glib::ustring> props = m_proxy->get_cached_property_names();
    Glib::Variant<{{ prop.variant_type }}> b;
    if (std::find(props.begin(), props.end(), "{{ prop.name }}") != props.end()) {
        m_proxy->get_cached_property(b, "{{ prop.name }}");
    } else {
        g_print("Todo: lookup value\n");
    }

could be changed to something like:

{{ prop.cpptype_out }} {{ class_name_with_namespace }}::{{ prop.name }}_get()
{
    Glib::Variant<{{ prop.variant_type }}> b;
    m_proxy->get_cached_property(b, "{{ prop.name }}");
    if (!b) {
        g_print("Todo: lookup value\n");
    }

I think... :)

mardy commented 5 years ago

Looks like that should work :-)

I would also like to change the signature of the getter method, to let the client know if the property exists or not. Throwing an exception could also be an option, though my personal preference would be adding an optional bool *ok = nullptr parameter.

martin-ejdestig commented 5 years ago

Hum, yes, adding an optional bool out parameter would be a way to keep API compatibility instead of making it return std::optional as I suggested in #83 .

Should probably also make the return value default to {{ prop.cpptype_out }} () so int:s and bools are zeroed.

But also, perhaps the "let caller know if property is cached" should be handled as a fix for #83 . :) (I just filed this to not forget about the unnecessary to extract names part.... a PR could of course fix both issues in two separate commits. :)