rs_settings

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

rs_settings

ravas
QString RS_Settings::readEntry(const QString& key,
                                 const QString& def,
                                 bool* ok) {
       
    // lookup:
    QVariant ret = readEntryCache(key);
    if (!ret.isValid()) {
                               
        QSettings s(companyKey, appKey);
    // RVT_PORT not supported anymore s.insertSearchPath(QSettings::Windows, companyKey);
               
                if (ok) {
                        *ok=s.contains(QString("%1%2").arg(group).arg(key));
                }
               
        ret = s.value(QString("%1%2").arg(group).arg(key), QVariant(def));
                cache[key]=ret;
    }

    return ret.toString();

}
What is the purpose of:

                if (ok) {
                        *ok=s.contains(QString("%1%2").arg(group).arg(key));
                }
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

dxli
if the ok pointer is supplied (instead of an invalid ok=nullptr, which is false when converted to boolean), .i.e.

     if(ok)

will check the ret part is read from an existing setting key, and return a true/false in the boolean pointed by *ok, .i.e.,
     if(ok) {
            *ok=s.contains(QString("%1%2").arg(group).arg(key));
     }


ravas wrote
QString RS_Settings::readEntry(const QString& key,
                                 const QString& def,
                                 bool* ok) {
       
    // lookup:
    QVariant ret = readEntryCache(key);
    if (!ret.isValid()) {
                               
        QSettings s(companyKey, appKey);
    // RVT_PORT not supported anymore s.insertSearchPath(QSettings::Windows, companyKey);
               
                if (ok) {
                        *ok=s.contains(QString("%1%2").arg(group).arg(key));
                }
               
        ret = s.value(QString("%1%2").arg(group).arg(key), QVariant(def));
                cache[key]=ret;
    }

    return ret.toString();

}
What is the purpose of:

                if (ok) {
                        *ok=s.contains(QString("%1%2").arg(group).arg(key));
                }
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

ravas
I don't understand what happens next, the purpose of setting "ok".
"ok" is not used again in the function.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

dxli
the *ok pointer is passed by the function caller.

this way, the function can return two values, the part by return, and the boolean by *ok. If the caller needs the ret part only, the pointer is set to nullptr, and the *ok part would be ignored.

Another similar example, to find the nearest Endpoints in rs_entitycontainer:

    RS_Vector RS_EntityContainer::getNearestEndpoint(const RS_Vector& coord, double* dist  = nullptr ) const;

if the caller wants to find the the nearest endpoint of entities, and the actual distance to it, call the function with two parameters, with the second one a valid pointer to a double.

If the caller only wants the nearest endpoint, and doesn't care about the distance, supply a nullptr to as the second parameter (the default value).

ravas wrote
I don't understand what happens next, the purpose of setting "ok".
"ok" is not used again in the function.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

dxli
another way of allowing more than one returned value:

std::pair<QString, bool> RS_Settings::readEntry(const QString& key,
                                 const QString& def) {
QString ret;
bool ok;
...

return std::pair<QString, bool>{ret, ok};
}

here ok is always calculated and returned.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

ravas
In reply to this post by dxli
谢谢

I understand now.

It's not possible with Python's name binding.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

dxli
considered a bad to thing to rely on raw pointers:

not the same as using reference, even though we can rewrite it to use reference:

QString readNumEntry(QString const& key, bool& ok)
{
if(ok){
   ok=...
}
}

It's trickier to do similar for getNearestEndPoint:

RS_Vector getNearestEndPoint(RS_Vector const& coord, double& dist)
{

if(dist<0.){
   dist=...
}
}




ravas wrote
谢谢

I understand now.

It's not possible with Python's name binding.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

ravas
I was unable to find usage of the "ok" parameter.

Even if it was used, it seems clearer for the check to be a separate function.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

dxli
This post was updated on .
something like:

QString ret=settings.contain(key)?settings.readEntry(key):QString("Default Value");


the current API is simpler:

QString ret=settings.readEntry(key, "Default Value", nullptr);

ravas wrote
I was unable to find usage of the "ok" parameter.

Even if it was used, it seems clearer for the check to be a separate function.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

ravas
What does passing nullptr accomplish?
QSettings::value is what is determining if the "def" argument is returned.
There doesn't seem to be any reason to use QSettings::contains.

It seems like the function can be simplified to:

QString RS_Settings::readEntry(const QString& key, const QString& def)
{
    if (!cache.count(key))
    {
        QSettings s(companyKey, appKey);
        QVariant ret = s.value(QString("%1%2").arg(group).arg(key), QVariant(def));
        cache[key]=ret;
    }
    return cache[key].toString();
}
The "ok" parameter and the readEntryCache function both seem worthless to me.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

dxli
We can simplify this in the master branch, the 2.1 to be

Let's review the Qt5 QSetings API, and see what we can follow:

http://doc.qt.io/qt-5/qsettings.html

ravas wrote
What does passing nullptr accomplish?
QSettings::value is what is determining if the "def" argument is returned.
There doesn't seem to be any reason to use QSettings::contains.

It seems like the function can be simplified to:

QString RS_Settings::readEntry(const QString& key, const QString& def)
{
    if (!cache.count(key))
    {
        QSettings s(companyKey, appKey);
        QVariant ret = s.value(QString("%1%2").arg(group).arg(key), QVariant(def));
        cache[key]=ret;
    }
    return cache[key].toString();
}
The "ok" parameter and the readEntryCache function both seem worthless to me.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

ravas
I think we can also simplify for cases like:
	RS_SETTINGS->beginGroup("/group");
	RS_SETTINGS->writeEntry("/key", value);
	RS_SETTINGS->endGroup();
One idea is to add a parameter for the group, thereby reducing that example to a single line.
The parameter could use the current group class member as a default,
so that the above syntax would still work for cases when we need to access more than one.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

dxli
To write key with Group:
writeEntry("Group/Key", value);

to write key with the current group:

writeEntry("Key" value);
or
writeEntry("./Key", value);


ravas wrote
I think we can also simplify for cases like:

<pre style="background-color: #EFEFFF; font-size: 14px;">
        RS_SETTINGS->beginGroup("/group");
        RS_SETTINGS->writeEntry("/key", value);
        RS_SETTINGS->endGroup();
</pre>

One idea is to add a parameter for the group, thereby reducing that example to a single line. <br>
The parameter could use the current group class member as a default, <br>
so that the above syntax would still work for cases when we need to access more than one.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

ravas
Oh right... because the group variable will be an empty string.
Reply | Threaded
Open this post in threaded view
|

Re: rs_settings

ravas
In reply to this post by dxli
After further consideration, it seems like a group parameter would make the function easier to understand,
and would also avoid the possibility of someone setting the group and then also using the "/group/key" syntax, which would produce "/group/group/key".