Я пытаюсь отфильтровать таблицу событий, чтобы получить текущие события и завершенные события, но pycharm продолжает подчеркивать мой код из-за дублирования кода. Как предотвратить дублирование кода цикла в двух приведенных ниже функциях? Еще лучше, как мне оптимально реорганизовать эти две функции? Спасибо

def get_ongoing_events():
    ongoing_events = Events.objects.filter(
            Q(event_begin_datetime__lte=current_time),
            Q(event_end_datetime__gt=current_time),
        )
    for event in ongoing_events:
        event.event_status="ongoing"
        event.save()

        event.venue.status="booked"
        event.venue.save()

        reserve_data = dict()
        reserve_data["sensor_id"] = event.venue.sensor_id
        reserve_data["status"] = event.venue.status

        return reserve_data


def get_complete_reservation():
    """
    Update reservations and sensors
    :return:
    """
    completed_events = Events.objects.filter(
            Q(reservation_begin_datetime__lt=current_time),
            Q(reservation_end_datetime__lte=current_time),
        )

    for event in completed_events:
        event.reservation_status="complete"
        event.save()

        event.venue.status="free"
        event.venue.save()
        reserve_data = dict()
        reserve_data["sensor_id"] = event.venue.sensor_id
        reserve_data["status"] = event.venue.status
        return reserve_data
```

Исключение дублирования

Всякий раз, когда у меня есть (почти) повторяющийся код, я просто смотрю на повторяющийся код. Части, которые хоть немного меняются, будут управляться аргументами функций и т. Д., Тогда как части, которые остаются такими же, будут оставлены более или менее как есть.

Сказав это, вот что я вижу:

• используемые запросы меняются, так что это аргументы функции;
• атрибуты обновляемого события изменяются, поэтому они являются аргументами функции;
• последний сгенерированный dict будет таким же, поэтому мы оставляем его прежним.

Вот предлагаемая модификация только из вашего кода:

def filter_and_update_events(Qs, event_changes, venue_changes):
    for event in Events.objects.filter(*Qs)
        for attr, new_value in event_changes.items():
            setattr(event, attr, new_value)
        event.save()
        for attr, new_value in venue_changes.items():
            setattr(event.venue, attr, new_value)
        event.venue.save()

        reserve_data = dict()
        reserve_data["sensor_id"] = event.venue.sensor_id
        reserve_data["status"] = event.venue.status
        return reserve_data

def get_ongoing_events():
    return filter_and_update_events(
        [
            Q(event_begin_datetime__lte=current_time),
            Q(event_end_datetime__gt=current_time),
        ],
        {"event_status": "ongoing"},
        {"status": "booked"},
    )

def get_complete_reservation():
    """
    Update reservations and sensors
    :return:
    """
    return filter_and_update_events(
        [
            Q(reservation_begin_datetime__lt=current_time),
            Q(reservation_end_datetime__lte=current_time),
        ],
        {"reservation_status": "complete"},
        {"status": "free"},
    )

Обратите внимание, что я использовал setattr документы для настройки атрибутов мероприятия и места проведения. Также обратите внимание, что моя новая функция имеет небольшое дублирование в циклах, но это самый простой способ справиться с тем фактом, что venue находится внутри event и нет очевидного способа использовать setattr разобраться с гнездованием. Если вам нужно расширить свою функцию, чтобы обновить еще больше вещей внутри event, то я бы также рекомендовал создать вспомогательную функцию, которая принимает event и «название настройки» и устанавливает его так, чтобы filter_and_update_events не нужно брать один словарь на объект внутри event.

Имеет ли это смысл?

Дублирование может быть не проблемой, но часто является сигналом о том, что что-то не на своем месте.

Этот повторяющийся фрагмент кода

    for event in completed_events:
        event.reservation_status="complete"
        event.save()

        event.venue.status="free"
        event.venue.save()
        reserve_data = dict()
        reserve_data["sensor_id"] = event.venue.sensor_id
        reserve_data["status"] = event.venue.status
        return reserve_data

Это проблематично, потому что действие, которое должно быть выполнено, когда событие продолжается / зарезервировано / и т. Д., Должно выполняться самим событием; вот почему status и venue объекты находятся внутри события, потому что они находятся в его домене. Я вижу что у тебя уже есть Events класс, может быть, есть еще Event класс?. Я предлагаю вам иметь класс Event, который бы делал что-то вроде этого

# Perhaps this constants could be stored in the Events class?
ONGOING = 'ongoing'
COMPLETE = 'complete'
VENUES_STATUS = {
    ONGOING: 'booked',
    COMPLETE: 'free'
}



class Event:
    def trigger(self, new_status, save=True):
        if new_status == ONGOING:
            self.event_status = ONGOING
            # What about reservation_status ?
        elif new_status == COMPLETE:
            self.reservation_status = COMPLETE
            # What about event_status ?
        else:
            pass  # you may have other statuses here?
        if new_status in VENUES_STATUS:
            self.venue.status = VENUES_STATUS[new_status]
        if save:
            self.save()
            self.venue.save()

    def data(self): # this could be moved to the Venue class, also a better name can be definitely found.
        return {
            "sensor_id": self.venue.sensor_id,
            "status": self.venue.status
        }

(Комментарии для вас больше, чем настоящие комментарии к коду).

При этом цикл становится (здесь я предполагаю, что вы пытаетесь собрать, потому что в вашем коде у вас есть возврат внутри цикла for; который всегда будет возвращать данные первого события.)

def get_complete_reservation():
    completed_events = Events.objects.filter(
        Q(reservation_begin_datetime__lt=current_time),
        Q(reservation_end_datetime__lte=current_time),
    )
    data = []

    for event in completed_events:
        event.trigger(ONGOING)
        data.append(event.data())
    return data

Название функции на самом деле не говорит о том, что вы делаете, вы не просто «получаете» события, вы запускаете транзакцию статуса от одного к другому, поэтому я бы переписал это так:

def trigger_new_status(selected_events, new_status):
    for event in selected_events:
        event.trigger(new_status)
    return [event.data() for event in selected_events]


def main():
    completed_events = Events.objects.filter(
        Q(reservation_begin_datetime__lt=current_time),
        Q(reservation_end_datetime__lte=current_time),
    )
    ongoing_events = Events.objects.filter(
        Q(event_begin_datetime__lte=current_time),
        Q(event_end_datetime__gt=current_time),
    )
    completed_events_data = trigger_new_status(completed_events, COMPLETE)
    ongoing_events_data = trigger_new_status(ongoing_events, ONGOING)

Другое дело: если у вас есть возможность изменить filter метод, лучше использовать функциональный подход:

def filter(self, filter_function):
    return [event for event in self.events if filter_function(event)]

и это будет использоваться как

events.filter(lambda e: e.reservation_end < today and e.reservation_begin> today)

хотя ваш код имеет дублирование, но он также является модульным. В зависимости от функциональности кода вы можете отказаться от этой модульности с меньшим дублированием.

как насчет объединения двух функций и возврата двух списков, как показано ниже? это устранит дублирование кода, однако компромиссом будет меньшая модульность кода.

Кроме того, вместо того, чтобы использовать словарь для хранения зарегистрированных и бесплатных идентификаторов датчиков, вы можете просто использовать списки с описательными именами, как это сделал я.

def get_booked_and_free_events():
ongoing_events = Events.objects.filter(
        Q(event_begin_datetime__lte=current_time),
        Q(event_end_datetime__gt=current_time),
    )
completed_events = Events.objects.filter(
        Q(reservation_begin_datetime__lt=current_time),
        Q(reservation_end_datetime__lte=current_time),
    )
for event in ongoing_events:
    event.event_status="ongoing"
    event.save()

    event.venue.status="booked"
    event.venue.save()

    booked_sensors = []
    booked_sensors.append(event.venue.sensor_id)

for event in completed_events:
    event.reservation_status="complete"
    event.save()

    event.venue.status="free"
    event.venue.save()

    free_sensors = []
    free_sensors.append(event.venue.sensor_id)
    
return booked_sensors, free_sensors