Коллеги, что вы думаете о данном коде?

[MikeR]

Этот табличный метод

static void updateParent(InventoryGuidInvoice inventoryGuidInvoice)
{
    InventoryGuid           current, parent;
    InventoryGuidInvoice    parentDistr;
    InvGuid                 InvGuid;
    ;

    InvGuid = inventoryGuidInvoice.InvGUID;

    ttsbegin;
    WHILE(TRUE)
    {
        SELECT FIRSTONLY current WHERE current.InvGuid == InvGUID;
        InvGUID = current.ParentInvGUID;

        SELECT FIRSTONLY parent WHERE current.ParentInvGUID == parent.InvGUID;

        if(parent)
        {
            //parentDistr.disableCache();
            SELECT FIRSTONLY
            FORUPDATE parentDistr
            WHERE parentDistr.InvGUID == InvGUID &&
                parentDistr.TransRecId == inventoryGuidInvoice.TransRecId
                //...........
                && parentDistr.JuridicalPersonId == inventoryGuidInvoice.JuridicalPersonId;

            if(parentDistr)
            {
                parentDistr.AMOUNTCUR += inventoryGuidInvoice.AmountCur;//слияние к предку
                //.......
                parentDistr.Update();
            }
            else
            {
                parentDistr.data(InventoryGuidInvoice);
                parentDistr.InvGUID = parent.InvGUID;
                parentDistr.insert();
            }
        }
        else
        {
            break;
        }

        current.clear();
    }
    ttscommit;
}

[ice]

а что вас в нем смущает? код пробегает по цепочке "вверх" и что то апдейтит или создает. Оформление конечно страдает

[MikeR]

Вы тоже так пишите?
Я хочу услышать мнения коллег, насколько так вообще корректно писать.
Или может у меня одного глаз замылился.

[ZVV]

Цитата:

Сообщение от MikeR

Вы тоже так пишите?
Я хочу услышать мнения коллег, насколько так вообще корректно писать.
Или может у меня одного глаз замылился.

Рекурсивный метод тут лучше бы смотрелся.
Ну по крайней мере я бы использовал рекурсию...

OFFTOP: не знал, что PL\SQL Formatter можно к Х++ прикрутить...

[MikeR]

Коллеги, вы не поверите, но суть в том, что цикл вообще не нужен.

WHILE(TRUE)

это стоит для того, что бы просто "запустить" выполнение.
Я подумал, что это очевидно.....

[samolalex]

Цитата:

Сообщение от MikeR

Коллеги, вы не поверите, но суть в том, что цикл вообще не нужен.

WHILE(TRUE)

это стоит для того, что бы просто "запустить" выполнение.
Я подумал, что это очевидно.....

Кстати, да. О чем и говорит firstonly. А теперь и мне стало интересно, зачем так было сделано - "запускать выполнение" таким образом?

[ZVV]

Цитата:

Сообщение от MikeR

Коллеги, вы не поверите, но суть в том, что цикл вообще не нужен.

WHILE(TRUE)

это стоит для того, что бы просто "запустить" выполнение.
Я подумал, что это очевидно.....

Так ли?
Он нужен для того что бы запускать и следующие итерации тоже, а не только первую.
Цикл будет выполнятся, кока не закончится дерево (не будет парента) и сработает брэйк.
Ну собственно такая эмуляция рекурсии...

[oip]

Цитата:

Сообщение от MikeR

Коллеги, вы не поверите, но суть в том, что цикл вообще не нужен.

WHILE(TRUE)

это стоит для того, что бы просто "запустить" выполнение.
Я подумал, что это очевидно.....

Почему? По-моему, это такой обход дерева. И без "while true" работать не будет. При каждой итерации меняется InvGUID и цикл прервется только тогда, когда не найдется parent. И "while true" при таком написании кода очень даже нужен. Понятно, что код можно переписать иначе.

[samolalex]

Если не касаться оформления, а именно верхних регистров SQL-операторов (лично для меня это непривычно), то while(true) в сочетании с break'ом - это очень "плохой тон", мягко говоря, в программировании в целом (не говоря уже о табличном методе в Ax), о чем, кстати, упоминал в своей книге "Совершенный код" Стив Макконнелл.
ИМХО, следует все же изучить связи таблиц и перестроить данный запрос на while select в сочетании с join'ом, пусть даже с внутренним select'ом.
Даже вложенный while не так бьет по глазам, как while(true), хотя это тоже не есть хорошо.

[gl00mie]

Слов нет, одни эмоции Вспомнились комментарии в ководстве к одному "шедевру":

Цитата:

Буааууеееееееееее! Ааааа! Глаза кровоточат!

Честно минут 15 тупил, пытаясь понять глубокий смысл этого кода и заодно - такой схемы данных. Что тут еще скажешь... "Пишите код так, как будто сопровождать его будет склонный к насилию психопат, который знает, где вы живете" (с) Стив Макконнелл

[oip]

Цитата:

Сообщение от MikeR

Кстати, а действительно кто-нибудь пишет рекурсию через

while (true)

в Dynamics Ax?

Да, в АОТе должно быть полно примеров. Например, в классе ReqCalc.

[MikeR]

Цитата:

Сообщение от oip

Да, в АОТе должно быть полно примеров. Например, в классе ReqCalc.

Олег, там используется перебор коллекции

while (mi.....more())

Это немного другое дело, здесь очевидно, надо было использовать запросную часть.

[oip]

Цитата:

Сообщение от MikeR

Олег, там используется перебор коллекции

while (mi.....more())

Это немного другое дело, здесь очевидно, надо было использовать запросную часть.

Ок. BOMDesignerCtrl\canMoveNode

    while (true)
    {
        if (bomTableMove.RecId == bomTableDrop.RecId)
        {
            dropParent          = tree.getParent(curParent);
            bomTableDropParent  = node2BOMTable.lookup(dropParent);
            return bomTableMoveParent.RecId != bomTableDropParent.RecId || dropParent == rootId;
        }
        else
        {
            curParent = tree.getParent(curParent);
            if (curParent == rootId)
                return true;
            bomTableDrop = node2BOMTable.lookup(curParent);
        }
    }

[MikeR]

Цитата:

Сообщение от oip

Ок. BOMDesignerCtrl\canMoveNode

Отыскал таки
По существу - ты бы сам рекомендовал при однозначной связи потомком и родителя делать через while (true), а не через рекурсию?

[db]

Спор ни о чем.
Например с помощью топора можно построить дом или проломить кому нибудь голову. Из этого можно сделать однозначное заключение о применимости топора в любых условиях?

Цитата:

Сообщение от MikeR

Отыскал таки
По существу - ты бы сам рекомендовал при однозначной связи потомком и родителя делать через while (true), а не через рекурсию?

Рекурсия может легко переполнить стек. Но это утверждение опять же ничего не значит в отрыве от конкретной задачи

[ice]

ни где не встречал требований, каким образом делать проход по дереву, так что выбранный программистом подход имеет место быть. и отлавливать ошибки в таком подходе гораздо легче, чем при использовании рекурсии. но рекурсия конечно красивей

[lvan]

с рекурсией поосторожнее, в 2012 она не поддерживается в CIL

[gl00mie]

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

[MikeR]

Большое всем спасибо за высказанные мнения.
Наверное тему все ж таки закрою, так как за меня взялись серьезные тролли.
Приходится объяснять очевидные вещи, а это грозит перейти в обсуждение теории относительности (кто вокруг кого крутится )
Мое мнение таково, что не зря существуют в серьезных системах определенные фреймворки и надо их знать, что бы было легко поддреживать код.
А так согласен, что писать можно лишь бы работало.
В сравнение с ремонтом машин - в сервис БМВ берут слесаря после ремонта Жигулей только после переатестации, так как ремонт отличается, хотя и там и там одни и те же гайки, но подход к ремонту отличен. Да можно вручить "местному спецу" отремонтировать и БМВ, но ездить на машине скорее всего будете не долго.

[mazzy]

MikeR, отвечу поскольку ты попросил.
В целом согласен с предыдущими авторами.

Прежде всего - насколько я понимаю, вопрос не об оформлении кода, а о сути метода. Метод по сути выполняет обход и обновление дерева. Причем почему-то достраивает дерево на лету. На человеческий язык пользовательской задачи этот метод я не могу перевести.

========================
С деревом в реляционных базах - плохо.
Я об этом писал http://axapta.mazzy.ru/lib/tree2/ (там ссылки есть на обсуждения у других)
Особенно плохо с универсальным деревом parent/child с произвольной глубиной.

Обход универсального дерева в реляционых базах никак не делается в один запрос.
Это по запросам.

Кроме того, работа с универсальным деревом с произвольной глубиной именно через бесконечный цикл и делается. Или через рекурсию. (Но рекурсия на сложных структурах в java выполняется сложно, поскольку в java все переменные передаются по ссылке)
Это по циклу/рекурсии.

по поводу ttsbegin/ttscommit - тоже согласен. для сохранения целостности правильно поставлено.

Цитата:

Сообщение от MikeR

Этот табличный метод

static void updateParent(InventoryGuidInvoice inventoryGuidInvoice)
{
    InventoryGuid           current, parent;
    InventoryGuidInvoice    parentDistr;
...
        SELECT FIRSTONLY current WHERE current.InvGuid == InvGUID;
...
        SELECT FIRSTONLY parent WHERE current.ParentInvGUID == parent.InvGUID;
...
}

можно было бы чуть пооптимизировать и сделать в один запрос.
но если у таблицы задано кэширование, то два простых запроса по одному ключу каждый будут брать данные из кэша самой Аксапты, что здорово сэкономит на запросах к SQL. Поэтому может быть и правильно, что два запроса.

====================
в общем: если в постановке задачи стояло универсальное дерево, то по-другому написать было сложно

другое дело, что деревья в реляционной базе - зло. и их просто не нужно допускать по возможности. А если уж необходимость в дереве есть, то рассмотреть возможность другого хранения ссылок (не parent/child). Почитай по ссылкам со страницы http://axapta.mazzy.ru/lib/tree2/