MarkupTable.checkCustAccount(), баг?

[ZVV]

Затрудняюсь с позиционированием вопроса, пусть будет "Прочий".
Просьба перенести при необходимости.

Указанный в сабже метод на таблице (Версии 3.0 - 2009, вроде не менялось)

boolean  checkCustAccount()
{
    boolean  ok = true;
    ;

    if (this.CustAccount)
    {
        ok = LedgerTable::checkAllowPosting(this.CustAccount);
    }

    if (this.CustType != MarkupType::LedgerAccount && (this.CustType != MarkupType::Item || !PurchParameters::find().PostChargeAccount))
    {
        if (this.CustAccount)
            ok = checkFailed("@SYS25165");
    }
    else
    {
        if (this.CustType != MarkupType::Item || !PurchParameters::find().PostChargeAccount)
            if (!this.CustAccount)
                ok = checkFailed("@SYS25989");
    }

    return ok;
}

Варианта 3, либо тут баг, либо лишний код, лио я вообще ничего не понимаю.
Поясню:
- В первый блок ( if (this.CustType != MarkupType::LedgerAccount && (this.CustType != MarkupType::Item || !PurchParameters::find().PostChargeAccount)) ) заходим, если тип Клиент/Поставщик, либо тип Номенклатура с НЕустановленной птичкой PostChargeAccount. В этом случае счёт задавать нельзя. С этим всё понятно.
- Во второй блок (else) мы заходим соответсвенно в остальных случаях (Тип = Бух.Счёт либо тип Номенклатура с установленной птичкой). Но на горизонте появляется этот второй IF (if (this.CustType != MarkupType::Item || !PurchParameters::find().PostChargeAccount)), вторая часть которого вообще в данном случае бессмысленна, т.к. с неустановленной галкой при типе Номенклатура мы в этот блок в принципе не зайдём.
Может конечно они ошиблись с отрицанием, но тогда весь этот IF во втором блоке - тавтология. Да и судя по соседнему методу checkCustType(), это не совсем верно...

Вообщем не совсем понятно с точки зрения функционала, как правильно...

[S.Kuskov]

Не вдаваясь в смысл проверок, могу предложить моё видение этого кода

if (A && B) // (1) сюда попадём если условие A = true и B = true
else if (B) // (2) сюда попадём если условие A = false, a B = true

получается для возникновения ошибки условие B должно быть всегда истино. Т.е. логично было бы написать так:

if (B)
{
    if (A) {/* (1) сюда попадём если условие A = true и B = true */}
    else   {/* (2) сюда попадём если условие A = false, a B = true */}
}

В итоге получаем, что-то вроде

    if (this.CustType != MarkupType::Item || !PurchParameters::find().PostChargeAccount)
    {
        if ((this.CustType != MarkupType::LedgerAccount) && this.CustAccount)
            ok = checkFailed("@SYS25165");
        if ((this.CustType == MarkupType::LedgerAccount) && !this.CustAccount)
            ok = checkFailed("@SYS25989");
    }

[ZVV]

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

То что конкрено этот код можно сократить, я описал сразу. Просто не хотелось бы упустить какую багу. Так что извините, но лучше уж "вдаваясь в смысл проверок"...

[SRF]

Рискну предположить, что custAccount должен быть заполнен обязательно только, если CustType = MarkupType::LedgerAccount, обязательно быть пустым, если CustType = MarkupType::CustVend, и может быть заполнено, а может быть и нет для CustType = MarkupType::Item. (вроде логичная предпосылка ).

Теперь разложим if - как это сделал S.Kuskov

Цитата:

if (A && B) // (1) сюда попадём если условие A = true и B = true
else
{
if (B) // (2) сюда попадём если условие A = false, a B = true
}

В нашем случае, мы попадаем в else :
либо A = false и B = true,
либо наоборот A = true, B = false,
либо A и B = false - одновременно;
где

A = this.CustType != MarkupType::LedgerAccount 
B = (this.CustType != MarkupType::Item || !PurchParameters::find().PostChargeAccount)

A и B = false - одновременно, говорят, что это тип номеклатура с галкой PostChargeAccount => можешь задавать CustAccount, а можешь и нет(вот тут не совсем ясно, обязательно ли должно быть заполнено поле CustAccount, если да, то ошибка в проверке, если может быть заполнено, а может быть и нет, то все вроде ок)

В случае, когда A или B - false, соответственно нужно проверять заполненость custAccount, когда CustType = MarkupType::LedgerAccount, т.е. когда A = false и B = true => писать в коде либо if (! A), либо if (B) - выбран второй вариант, почему так, сказать сложно, конечно нагляднее написать проверку :

if (this.CustType = MarkupType::LedgerAccount)

но работать будет одинаково.