3
\$\begingroup\$

There must surely be a nicer way of writing this.

The wrinkle of complexity is that prev_seq or next_seq might be an empty string if the user moves the question to the first or last position, but it could also validly be '0.0' so I can't just rely on 'falsiness'.

I hope it's clear what I'm trying to achieve from the below (with the comments).

How can I improve this?

def update_sequence(self):

    prev_seq = self.request.get('prev_seq')
    next_seq = self.request.get('next_seq')
    question_id = int(self.request.get('id'))
    question = db.get(db.Key.from_path('Questions', question_id))

    try:
        # to slot a question between two numbers we compute the midpoint
        # for, example, to insert a question between 2.1 and 3.4, the
        # question would get the sequence of 2.75
        new_position = (float(prev_seq) + float(next_seq)) / 2
    except ValueError:
        # if we get here, either prev_seq or next_seq was not submitted
        # in the request which means the user pushed the question to the
        # first or last position.
        try:
            float(prev_seq)
        except ValueError:
            # in this case the question has been moved to the first
            # position so we should make:
            # the new position = the next position - 1
            new_position = float(next_seq) - 1
        try:
            float(next_seq)
        except ValueError:
            # in this case the question has been moved to the last position
            # so we should make:
            # the new position = the previous position + 1
            new_position = float(prev_seq) + 1

    question.sequence_float = new_position
    question.put()
\$\endgroup\$
  • \$\begingroup\$ Have you tried if prev_seq == "":? That gives a different result for "" and 0. \$\endgroup\$ – jcfollower Jan 9 '14 at 20:52
1
\$\begingroup\$
def get_new_position(prev_seq, next_seq):

    if next_seq == "":
        new_position = float(prev_seq) + 1
    elif prev_seq == "":
        new_position = float(next_seq) - 1
    else:
        new_position = (float(prev_seq) + float(next_seq)) / 2

    return new_position
\$\endgroup\$
  • \$\begingroup\$ yes, this is much nicer. for some reason I was trying to avoid testing equality with an empty string, I have no idea why. \$\endgroup\$ – Horselover Fat Jan 10 '14 at 11:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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