Почему это не так, если заявление работает правильно

Question

Этот код предназначен для очистки после повторной гидратации почтового ящика (из Symante c Enterprise Vault). Мы берем индекс моментального снимка MessageId и ConversationId всех элементов в почтовом ящике до повторной гидратации.

После повторной гидратации этот код

    if (string.Equals(item.ItemClass, "IPM.Note.EnterpriseVault.Shortcut", StringComparison.InvariantCulture) || ((existingIds.Any(x => x.ConversationId == item.ConversationId.ToString()) == false || (item.ItemClass == "IPM.Appointment" && existingIds.Any(x => x.MessageId == item.Id.ToString()) == false) && item.DateTimeReceived < snapshotDate)))
    {
        item.Delete(DeleteMode.HardDelete);
    }

должен удалить

1. Любые элементы, которые имеют ItemClass «IPM.Note.EnterpriseVault.Shortcut»
2. Любые элементы, которые имеют ItemClass «IPM.Appointment», где Id не является в списке existingIds из MessageId с, если они не были получены после `snapshotDate
3. Любые другие элементы, где ConversationId отсутствует в списке existingIds, если они не были получены после snapshotDate.

После запуска этого кода пользователь сообщил, что потерял какое-то электронное письмо, полученное после snapshotDate, так что, похоже, я неправильно понял утверждение if! :( Может кто-нибудь сказать мне, пожалуйста, что я ошибся (или способ, которым я могу разбить это, чтобы понять это лучше) и что этот код на самом деле сделал, чтобы я мог сообщить пользователю, что было потеряно. Я знаю, что ИЛИ, как известно, трудно написать, и я думаю, что где-то допустил ошибку в скобках, но я просто не вижу этого.

Answer 1

Я бы порекомендовал вам разделить, если проверка на локальные функции будет намного легче отлаживать.

 if (IsShourtcut(item) || NotExistingAppItment(item) || ExistingConversation(item) && IsReceivedBeforSnapshot(item)) 
    {
            // to delete 
    }

bool IsShourtcut(Item item) => string.Equals(item.ItemClass, "IPM.Note.EnterpriseVault.Shortcut", StringComparison.InvariantCulture);
bool NotExistingAppItment(Item item) => item.ItemClass == "IPM.Appointment" && existingIds.All(x => x.MessageId != item.Id.ToString());
bool ExistingConversation(Item item) => existingIds.All(x => x.ConversationId != item.ConversationId.ToString();
bool IsReceivedBeforSnapshot(Item item) => item.DateTimeReceived < snapshotDate;

Answer 2

Я считаю, что самый простой способ взглянуть на проблему такого рода - это использовать множество разрывов строк и отступов. Я добавляю разрыв и увеличиваю отступ после каждого ( (кроме тривиального ()), помещаю соответствующие ) s ниже их соответствующей пары и помещаю операторы в отдельные строки между элементами, к которым они присоединяются:

if (
    string.Equals(
        item.ItemClass,
        "IPM.Note.EnterpriseVault.Shortcut", StringComparison.InvariantCulture
    )
    ||
    (
        (
            existingIds.Any(
                x => x.ConversationId == item.ConversationId.ToString()
            ) == false
            ||
            (
                item.ItemClass == "IPM.Appointment"
                &&
                existingIds.Any(
                    x => x.MessageId == item.Id.ToString()
                ) == false
            )
            &&
            item.DateTimeReceived < snapshotDate
        )
    )
)
{
    item.Delete(DeleteMode.HardDelete);
}

Я могу сразу заметить две вещи - есть пара скобок, которая просто содержит другую пару, и у нас есть && и ||, встречающиеся на одном и том же "уровне", поэтому мы полагаемся на приоритет оператора.

Я предполагаю, что вы хотели && за пределами внутренних скобок, чтобы он применялся как к проверке назначения, так и к существующей проверке идентификаторов. Например, вместо этого:

if (
    string.Equals(
        item.ItemClass,
        "IPM.Note.EnterpriseVault.Shortcut", StringComparison.InvariantCulture
    )
    ||
    (
        (
            existingIds.Any(
                x => x.ConversationId == item.ConversationId.ToString()
            ) == false
            ||
            (
                item.ItemClass == "IPM.Appointment"
                &&
                existingIds.Any(
                    x => x.MessageId == item.Id.ToString()
                ) == false
            )
        )
        &&
        item.DateTimeReceived < snapshotDate
    )
)
{
    item.Delete(DeleteMode.HardDelete);
}

(Как только вы подтвердите, что все соответствует, как вам нужно, вы можете свернуть обратно на меньшее количество строк)

Answer 3

Оба других ответа на этот вопрос были чрезвычайно полезны, и я соответственно проголосовал. Однако я подумал, что опубликую свое окончательное решение по этому вопросу, вдохновленное { ссылка } и { ссылка }, особенно сильно опираясь на использование локальных функций в соответствии с рекомендациями OxQ. Теперь я написал модульные тесты (которые я должен был сделать раньше), и все они проходят с использованием моего нового метода, тогда как пять из них раньше не проходили. Я считаю, что этот код более точно соответствует заявленной проблеме и довольно прост для чтения и понимания. Мне пришлось использовать скалярные значения, а не весь объект Item, поэтому я могу выполнить его модульное тестирование, поскольку управляемый API-интерфейс веб-служб Exchange не использует интерфейсы.

switch (itemClass)
{
    case "IPM.Note.EnterpriseVault.Shortcut":
        return true;
    case "IPM.Appointment":
        return NotExistingMessage() && IsReceivedBeforeSnapshot();
    default:
        return NotExistingConversation() && IsReceivedBeforeSnapshot();
}

bool NotExistingMessage() => existingIds.All(x => x.MessageId != messageId);
bool NotExistingConversation() => existingIds.All(x => x.ConversationId != conversationId);
bool IsReceivedBeforeSnapshot() => dateTimeReceived < uploadDate;