2

Which solution for my small problem would you consider a better style? Is there another better option that I am missing?

The logic is simple: Process data from a source until you get the END token.

I have couple of imperfect ideas:

line = read_from_source()
while line != "END":
    process(line)
    line = read_from_source()

I am actually calling read_from_source() twice when the natural language description of the issue is calling it once. Potential for a maintenance bug when someone changes one place and forgets the other.

while True:
    line = read_from_source()
    if line == "EOF": break
    process(line)

I somehow dislike this as the loop condition doesn't stand out clearly from the code when glancing at it.

while line = read_from_source() != "END":
    process(line)

Concise, but I find it tough to raed when assignment and equality operator are on the same line.

  • 4
    (1) Which style is preferred is largely opinion-based. This site is about questions and answers on software engineering concepts, not for discussion or polls. (2) Your last example won't work in Python: assignment is a statement, not an expression. I suppose that the most elegant solution would be to turn your read_from_source function into a generator, then use a for-loop for line in read_from_source(): ... – amon Mar 8 '17 at 14:32
  • (2) indeed doesn't work :( (1) I thought that the question about which code is more readable, less error-prone, more maintainable, more x-able etc.. (aka "good style") is on topic here – Kuba Mar 8 '17 at 14:40
  • As to the generator - I agree. What I am seeking with this question is how to write this generator. Imagine that this code snipped is the generator_function and process(line) is a yield line. – Kuba Mar 8 '17 at 14:48
2

You can also do a break instead.

while True:
    line = read_from_source()
    if line == 'END':
        break
    process(line)

This is not unusual in Python. While many would not like the style, and be uncomfortable with the while True part, I prefer it over not reading two places. You could wrap it as a generator:

def lines_from_source(source):
    while True:
        line = read_from_source(source)
        if line == 'END':
            break
        yield line

And then do:

for line in lines_from_source(my_source):
    process(line)
1

Separate the concept of 'being at the end of the file' from 'reading data from a file'

var fileStream = File.Open(filename);
while(!fileStream.EOF)
{
    var line = fileStream.ReadLine();
    process(line);
}

version for a service

var serviceWrapper= new ServiceWrapper(service);
while(!serviceWrapper.IsAtEndOfSection)
{
    var line = serviceWrapper.ReadLine();
    process(line);
}

Or really, if you have complex data you should deserialise to an object

Class Data
{
    List<Section> Sections;
}

Class Section
{
    list<string> Lines;
}
  • It's not relevant to my case as I'm not reading from a file, but from a service which has only the service.readline() method. Also after one end there will be more messages from the same service so this is rather the end of a multiline message, then the end of a source. – Kuba Mar 8 '17 at 14:37
  • the same solution applies. add a wrapper to your service with EOF logic as required – Ewan Mar 8 '17 at 14:52
1

Read in a loop and if that's the end - break the loop.

while True:
    line = read_from_source()
    if line == "EOF":
        break
    process_the_line()

Replace True with some other condition if you expect your source to break earlier than "EOF"

  • Actually this was the example I put into my original question, but somehow I ended up with a copy-paste mistake and version 1 and 2 of my code is identical. ugh. This was the part I referred to as "the loop condition doesn't stand out when glancing at the code" – Kuba Mar 8 '17 at 15:13
  • Because it's not the loop condition here, it's break condition 8) It's also how you write a do-while loop in python. – aragaer Mar 8 '17 at 15:18
0

Not familiar with Python syntax, but you will get the idea

var line = "";
while (line != "END")
{
    var line = fileStream.ReadLine();
    if (line != "END")
    {
        process(line);
    }
}

Now, the drawback of this method is that you have two checks for line != "END" The alternative could be that you just make an infinite loop, and break out of it when line == "END". Less pretty, but more efficient. The prettier solution would be to modify process method to handle the "END" string properly, in which case you do not need to have a check within the loop.

0

You could make your second option clearer:

doneReading = false    
while not doneReading:
    line = read_from_source()
    if line == "EOF":
        doneReading=true;
    else:
        process(line)

Now your condition has a name, can handle more complex logic if you need it to. Also, it gets rid of an unnecessary break, so that's good ;)

Your first option really isn't that bad, but I understand you don't like calling read_from_source() in two different places. Your third option is a little hard to read, what with the multiple = in the same statement, I would avoid that style when possible.

0

Do not write loops, the loops are already written for you! Use them.

from itertools import takewhile, count

legit_line = lambda line: line != 'END'
result = [process(line) 
          for line in takewhile(legit_line, (read_from_source() for _ in count()))]

This is, of course, a bit of extremism, but it only looks weird because generating an endless sequence of read_from_source calls is not very natural in Python. You can use an explicit loop for it, though, still factoring out the check:

def all_lines():
  while True:
    yield read_from_source()

for line in takewhile(legit_line, all_lines):
  # Here come only non-END lines again.

Not the answer you're looking for?Browse other questions tagged or ask your own question.