How can I better structure this code?

Question

I have an lxml.objectify data structure I get from a RESTful web service. I need to change a setting if it exists and create it if it doesn't. Right now I have something along the lines of the following, but I feel like it's ugly. The structure I'm looking in has a list of subelements which all have the same structure, so I can't just look for a specific tag unfortunately.

thing_structure = lxml.objectify(get_from_REST_service())
found_thing = False
if thing_structure.find('settings') is not None:
    for i, foo in enumerate(thing_structure.settings):
        if foo.is_what_I_want:
            modify(thing_structure.settings[i])
            found_thing = True
if not found_thing:
    new = lxml.etree.SubElement(thing_structure, 'setting')
    modify(new)

send_to_REST_service(thing_structure)

Answer 1

Overall, the structure isn't too bad (assuming you need to call modify on 1+ items in the settings -- if "just one", i.e., if the is_what_I_want flag is going to be set for one setting at most, that's of course different, as you could and should use a break from the for loop -- but that's not the impression of your intentions that I get from your Q, please clarify if I've mistead!). There's one redundancy:

for i, foo in enumerate(thing_structure.settings):
    if foo.is_what_I_want:
        modify(thing_structure.settings[i])
        found_thing = True

Having i and using it to get again the same foo is no use here, so you can simplify to:

for foo in thing_structure.settings:
    if foo.is_what_I_want:
        modify(foo)
        found_thing = True

You'd only need the index if you were to rebind the item, i.e., perform an assignment such as thing_structure.settings = whatever. (BTW, a name other than foo wouldn't hurt;-).

Answer 2

I would write this:

thing_structure = lxml.objectify(get_from_REST_service())
if thing_structure.find('settings') is not None:
    foos = [foo for foo in thing_structure.settings if foo.is_what_I_want]
        or [lxml.etree.SubElement(thing_structure, 'setting')]
    for foo in foos:
       modify(foo)
send_to_REST_service(thing_structure)

I don't care for is not None and eliminate it where I can. It it is possible here, I would write:

if thing_structure.find('settings'):