D15520: Upgrade SimpleIpV4AddressValidator and SimpleIpV6AddressValidator

Pino Toscano noreply at phabricator.kde.org
Tue Sep 18 06:17:41 BST 2018


pino added a comment.


  - please remove all the empty `initTestCase()` in tests

INLINE COMMENTS

> simpleiplistvalidator.cpp:27-28
> +SimpleIpListValidator::SimpleIpListValidator(QObject *parent,
> +                                                       AddressStyle style,
> +                                                       AddressType type)
> +    : QValidator(parent)

indentation

> simpleiplistvalidator.cpp:64
> +    // Split the incoming address on commas possibly with spaces on either side
> +    QStringList addressList = address.split(QRegularExpression("\\s*,\\s*"));
> +

once again, simple char split with trim, please; you can use both splitRef + trimmed with QStringRef, so there is almost no waste of memory
using a regexp for a simple task like this is that it's way slower than using a simple character as separator

> simpleiplistvalidator.cpp:69
> +    int localPos = 0;
> +    int i = 0;
> +    QValidator::State result = QValidator::Acceptable;

the variable `i` is only incremented, and never used

> simpleipv4addressvalidator.cpp:93
>      // lets check address parts
> -    Q_FOREACH (const QStringRef &part, addrParts) {
> +    for (const QStringRef &part : addrParts) {
>          if (part.isEmpty()) {

while this change is OK, please do not mix it together with this patch

> simpleipv6addressvalidator.cpp:48-50
> +    if (QValidator::Invalid == checkWithInputMask(address, pos))
>          return QValidator::Invalid;
>  

unrelated changes

> simpleipv6addressvalidator.cpp:112
>      int i = 1;
> -    Q_FOREACH (QString part, addrParts) { // krazy:exclude=Q_FOREACH 
> +    for (QString part : addrParts) { // krazy:exclude=Q_FOREACH 
>          if (part.isEmpty() && i < number) {

as above, unrelated change

> simpleipv4test.cpp:23
> +#include <QTest>
> +#include <QDir>
> +

unused

> simpleipv6test.cpp:23
> +#include <QTest>
> +#include <QDir>
> +

unused

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15520

To: andersonbruce, jgrulich, pino
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180918/dd9187bd/attachment-0001.html>


More information about the Plasma-devel mailing list