openlp-core team mailing list archive

[Ken Roberts <alisonken1@xxxxxxxxx>]

Clarification: Looking at the loop, the question I have is "why is the
second check out of 6 checks inconsistent with the control structure?"

That whole section is predicated on checking the beginning of a line.
If the beginning of the line has already been matched with the first
"if", why is it being rechecked for a different value?

Original control statement:
 for line in text_list:
            if line.startswith('[S '):
                ...
            if line.startswith('Title='):
                ...
            elif line.startswith('Author='):
                ...
            elif line.startswith('Copyright='):
                ...
            elif line.startswith('Themes='):
                ...
            elif line.startswith('Fields='):
                ...
            elif line.startswith('Words='):
                ...
            # Unhandled usr keywords: Type, Version, Admin, Keys

When looking at code like that, it gives the impression that someone
is not paying attention to their control loop.

On Sat, Jan 10, 2015 at 8:04 PM, Ken Roberts <alisonken1@xxxxxxxxx> wrote:
> Is there a reason why the original 'if' statement on the second check
> is not consistent with the purpose of the rest of the checks?
>
> On Sat, Jan 10, 2015 at 11:40 AM, Tim Bentley <tim.bentley@xxxxxxxxx> wrote:
>> Review: Needs Fixing
>>
>>

<snip>
>>
>>>          ``[File]``
>>>              USR file format first line
>>> @@ -160,7 +163,7 @@
>>>                      self.ccli_number = ccli[4:].strip()
>>>                  else:
>>>                      self.ccli_number = ccli[3:].strip()
>>> -            if line.startswith('Title='):
>>
>> Why change when the previous if can change the value of line.
>>
>>> +            elif line.startswith('Title='):
>>>                  self.title = line[6:].strip()
>>>              elif line.startswith('Author='):
>>>                  song_author = line[7:].strip()
<snip>
-- 
- Ken
Registered Linux user 296561
Slackin' since 1993
Slackware Linux (http://www.slackware.com)

https://code.launchpad.net/~alisonken1/openlp/ccli-usr-bin-fix/+merge/246037
Your team OpenLP Core is subscribed to branch lp:openlp.