andersonbruce added inline comments. INLINE COMMENTS
> jgrulich wrote in simpleiplistvalidator.cpp:39 > In both validator constructors (mean also for IPv6 one), you can directly > pass "style" param from contructor, given both enums seem to be identical. They are the same now but initially they were not (I just added the WithPort option to IPv6) and my thought was that they may not necessarily stay identical in the future. If someone goes in and changes, say, the IPv4 version without realizing it affects the list validator, this code still works. > pino wrote in simpleiplistvalidator.cpp:62 > no need for a regexp, please use `QString::split()` with > `QString::SkipEmptyParts` I used the regex so that it would split on commas with or without spaces around them and how I read SkipEmptyParts is that it will drop something between two adjacent commas which is not what I was trying to do. Now I know that I could use "trimmed()" on the resulting strings to strip off any remaining spaces but I don't see any advantage to either using "trimmed()" every time I use one of the resulting strings or doing it once and creating another local copy of the strings for this purpose. It seemed to me that doing everything in one step was a just as viable approach unless there is something inherently bad with splitting on a regex that I am not aware of, especially given that I commented it to explain what it is doing. > pino wrote in simpleiplistvalidator.cpp:67 > set (later on), but never use I don't understand this comment. 'result' is set here to Acceptable, it can be modified to Intermediate at line 102 but if all the addresses are valid it needs to have been set initially to Acceptable for the return at line 106 to be correct. > pino wrote in simpleiplisttest.cpp:53 > `QVERIFY(foo == bar)` -> `QCOMPARE(foo, bar)`, it applies to all the checks > in this file Any particular reason you think this is superior? REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D15521 To: andersonbruce, jgrulich, pino Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
