openlp-core team mailing list archive

[Martin Thompson <mjthompson@xxxxxxx>]

Review: Needs Fixing
Erm, this is a bit late, sorry!

Have to be a bit careful with removing "is not None", for occasions
where '0' is a valid and distinct value from None.

For example:
=== modified file 'openlp/core/utils/confighelper.py'
--- openlp/core/utils/confighelper.py   2009-09-29 02:54:32 +0000
+++ openlp/core/utils/confighelper.py   2009-11-04 01:20:28 +0000
@@ -57,7 +57,7 @@
         if reg.has_value(section, key):
             return reg.get_value(section, key, default)
         else:
-            if default is not None:
+            if default:
                 ConfigHelper.set_config(section, key, default)
             return default

If default is 0 this code will not actually set the default value any more...

On my (admittedly not exhaustive) scan through the merge, this was the
only one I spotted...

However, something similar happens for strings which are empty (ie "")
eg:

-        if opensong.title is not None:
+        if opensong.title:
             self.set_title(opensong.title)

If there is a blank title, does it matter that the set_title() call
will not be made?  There are quite a number of string comparisons.

Cheers,
Martin

-- 
https://code.launchpad.net/~meths/openlp/trivialfixes/+merge/14397
Your team OpenLP Core is subscribed to branch lp:openlp.