1
\$\begingroup\$

We get the most relevant feedback about a doctor depending on the speciality, intervention_type and sub_intervention_type a patient is going to use.

The ordering is important here, we match feedback on speciality and sub_intervention_type first. If that's not available, try less specific matches.

I wonder if there is a better/prettier way to do this.

I thought about having each condition (e.g. feedback.intervention_type == intervention_type) as a method of a class.

def get_feedback_condition_method_names(self):
    return [method_name for method_name in dir(self) if method_name.startswith("feedback_condition")]

We could then loop through get_feedback_condition_method_names() and execute the methods in a loop. This way it's easy to add additional conditions, but we probably won't need to add additional conditions, so the below "functional" would be OK and the above would be overkill. Feedback welcome.

Code for review:

def get_doctor_feedback(*, doctor, speciality, intervention_type, sub_intervention_type):
    feedbacks = DoctorFeedback.objects.filter(doctor=doctor)
    result = [
        feedback for feedback in feedbacks
        if feedback.speciality == speciality and feedback.sub_intervention_type == sub_intervention_type
    ]
    if not result:
        result = [
            feedback for feedback in feedbacks
            if feedback.sub_intervention_type == sub_intervention_type
        ]
    if not result:
        result = [
            feedback for feedback in feedbacks
            if feedback.intervention_type == intervention_type
        ]
    if not result:
        result = [
            feedback for feedback in feedbacks
            if feedback.speciality == speciality
        ]
    if not result:
        result = feedbacks
    if not result:
        return doctor.default_feedback
    return result.sort(key=lambda feedback: feedback.rating)
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

You can use Python's or operator to chain the feedback result candidates and return the first non-empty list:

def get_doctor_feedback(*, doctor, speciality, intervention_type, sub_intervention_type):
    feedbacks = DoctorFeedback.objects.filter(doctor=doctor)

    result = get_feedback_by_priority(
        feedbacks,
        speciality=speciality,
        intervention_type=intervention_type,
        sub_intervention_type=sub_intervention_type
    )

    if not result:
        return doctor.default_feedback

    return sorted(result, key=lambda feedback: feedback.rating)


def get_feedback_by_priority(
        feedbacks,
        *,
        speciality,
        intervention_type,
        sub_intervention_type
):
    """Return first matching feedback list."""

    return [
        feedback for feedback in feedbacks
        if feedback.speciality == speciality
        and feedback.sub_intervention_type == sub_intervention_type
    ] or [
        feedback for feedback in feedbacks
        if feedback.sub_intervention_type == sub_intervention_type
    ] or [
        feedback for feedback in feedbacks
        if feedback.intervention_type == intervention_type
    ] or [
        feedback for feedback in feedbacks
        if feedback.speciality == speciality
    ] or feedbacks

And since the list comprehensions only differ by their condition, we can simplify this to:

def get_feedback_by_priority(
        feedbacks,
        *,
        speciality,
        intervention_type,
        sub_intervention_type
):
    """Return first matching feedback list."""

    for condition in [
            lambda feedback: (
                feedback.speciality == speciality
                and feedback.sub_intervention_type == sub_intervention_type
             ),
             lambda feedback: feedback.sub_intervention_type == sub_intervention_type,
             lambda feedback: feedback.intervention_type == intervention_type,
             lambda feedback: feedback.speciality == speciality
    ]:
        if candidate := [
                feedback for feedback in feedbacks
                if condition(feedback)
        ]:
            return candidate

    return feedbacks
\$\endgroup\$
2
\$\begingroup\$

This seems like a decent candidate for a while loop, since the checks on each if not result are almost identical:

def get_doctor_feedback(*, doctor, speciality, intervention_type, sub_intervention_type):
    feedbacks = DoctorFeedback.objects.filter(doctor=doctor)

    # store your lookups in an iterator that you advance in the loop
    attrs = iter((
        ('sub_intervention_type', sub_intervention_type),
        ('intervention_type', intervention_type),
        ('speciality', speciality),
    ))

    result = [
        feedback for feedback in feedbacks
        if feedback.speciality == speciality and feedback.sub_intervention_type == sub_intervention_type
    ]

    while not result:
        try:
            attr, value = next(it_attrs)
        except StopIteration:
            break

        result = [
            feedback for feedback in feedbacks 
            if getattr(feedback, attr, None) == value
        ]
    
    if not result and not feedbacks:
        return doctor.default_feedback
    elif not result:
        result = feedbacks

    return result.sort(key=lambda feedback: feedback.rating)

Bug

return result.sort returns None, return sorted(result, key... instead:

def get_doctor_feedback(*, doctor, speciality, intervention_type, sub_intervention_type):
    ~snip~

    return sorted(result, key=lambda feedback: feedback.rating)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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