Хелп! Объясните мне люди, я чего то не понимаю в этом мире.

Код ниже я считаю откровенным гвнокодом. Не могу никак доказать коллеге что это так.
Но может я ошибаюсь?

/// <summary>
/// Clear and places a specified data on the system Clipboard and accepts updater for additional clipboard updating.
/// </summary>
/// <param name="clipboardDataUpdater">The clipboard data updater.</param>
/// <exception cref="ArgumentNullException">clipboardDataUpdater</exception>
public static void SetData(Action<DataObject, Action> clipboardDataUpdater)
{
    if (clipboardDataUpdater == null)
    {
        throw new ArgumentNullException(nameof(clipboardDataUpdater));
    }

    Clipboard.Clear();
    var data = new DataObject();

    clipboardDataUpdater(data, () => Clipboard.SetDataObject(data, true));
}

Здравствуйте, barn_czn, Вы писали:

_>Хелп! Объясните мне люди, я чего то не понимаю в этом мире.

_>Код ниже я считаю откровенным гвнокодом. Не могу никак доказать коллеге что это так.
_>Но может я ошибаюсь?

А что здесь говнокодного? Запутанная логика размещения в клипборде?

Здравствуйте, barn_czn, Вы писали:

_>Хелп! Объясните мне люди, я чего то не понимаю в этом мире.

_>Код ниже я считаю откровенным гвнокодом. Не могу никак доказать коллеге что это так.
_>Но может я ошибаюсь?

Может быть. Попробуй объяснить, что тебе кажется тут не так?

Здравствуйте, barn_czn, Вы писали:

_>Код ниже я считаю откровенным гвнокодом. Не могу никак доказать коллеге что это так.
Да ладно тебе, код конечно не очень, но явно не самый худший образчик. Ничего смертельного. У коллеги может просто не хватать навыков и опыта, что бы понимать в чем он не прав.
_>Но может я ошибаюсь?
Скорее кидаешься в крайности. Проявляешь слишком много протекционизма. Мягче с коллегами надо.

Здравствуйте, Sharowarsheg, Вы писали:

S>Здравствуйте, barn_czn, Вы писали:

_>>Хелп! Объясните мне люди, я чего то не понимаю в этом мире.

_>>Код ниже я считаю откровенным гвнокодом. Не могу никак доказать коллеге что это так.
_>>Но может я ошибаюсь?

S>Может быть. Попробуй объяснить, что тебе кажется тут не так?

1. Action<DataObject, Action> — вот такие делегаты, принимающие делегата вам мозг не выносят? А ведь метод то в 3 строки, т.е. делает чтото простое.
2. clipboardDataUpdater — вызывается в конце кода! Проще чтобы SetData вернул что надо, а делегат вызвать следом.

Здравствуйте, m2l, Вы писали:

m2l>Здравствуйте, barn_czn, Вы писали:

_>>Код ниже я считаю откровенным гвнокодом. Не могу никак доказать коллеге что это так.
m2l>Да ладно тебе, код конечно не очень, но явно не самый худший образчик. Ничего смертельного. У коллеги может просто не хватать навыков и опыта, что бы понимать в чем он не прав.
_>>Но может я ошибаюсь?
m2l>Скорее кидаешься в крайности. Проявляешь слишком много протекционизма. Мягче с коллегами надо.

Да не, ну этот метод считают крайне полезным, его в библиотеку тянут ).
Хрен с ним коллегой, мне за такую библиотеку стыдно.

Здравствуйте, AlexRK, Вы писали:

ARK>Здравствуйте, barn_czn, Вы писали:

_>>Хелп! Объясните мне люди, я чего то не понимаю в этом мире.

_>>Код ниже я считаю откровенным гвнокодом. Не могу никак доказать коллеге что это так.
_>>Но может я ошибаюсь?

ARK>А что здесь говнокодного? Запутанная логика размещения в клипборде?

Логика? три строчки, даже без ветвлений, можно называть логикой? )

Здравствуйте, barn_czn, Вы писали:

ARK>>А что здесь говнокодного? Запутанная логика размещения в клипборде?

_>Логика? три строчки, даже без ветвлений, можно называть логикой? )

Можно. Да и ветвление там есть, пусть и формально.

Здравствуйте, AlexRK, Вы писали:

ARK>Здравствуйте, barn_czn, Вы писали:

ARK>>>А что здесь говнокодного? Запутанная логика размещения в клипборде?

_>>Логика? три строчки, даже без ветвлений, можно называть логикой? )

ARK>Можно. Да и ветвление там есть, пусть и формально.

где ты увидел ветвление?? в if (clipboardDataUpdater == null) ?

Ааа.. ну тогда да. Тогда ты тоже часть того мира который я не понимаю )

Здравствуйте, barn_czn, Вы писали:

_>Ааа.. ну тогда да. Тогда ты тоже часть того мира который я не понимаю )

Ну, если судить по ответам в этой теме, то скорее ты принадлежишь к миру, который мало кто понимает.

Здравствуйте, barn_czn, Вы писали:

m2l>>Скорее кидаешься в крайности. Проявляешь слишком много протекционизма. Мягче с коллегами надо.

_>Да не, ну этот метод считают крайне полезным, его в библиотеку тянут ).
_>Хрен с ним коллегой, мне за такую библиотеку стыдно.
Мне его несколько раз пришлось прочесть, чтобы догнать какие проблемы. И то не факт, что наши мнения совпадают. Т.е. с первого взгляда ничего не цепляет — код скорее несколько неоптимален/нелогичен, чем прямо вот днище.
И за библиотеку толку переживать нет. Сравнивая с тем, что порой мелькает в топовых либах, здесь тоже все норм. Так что тянут — забей.
Не трать силы на споры с коллегами. Лучше качай свои скилы, пиши лучший код и в том же духе.

Здравствуйте, barn_czn, Вы писали:

S>>Может быть. Попробуй объяснить, что тебе кажется тут не так?

_>1. Action<DataObject, Action> — вот такие делегаты, принимающие делегата вам мозг не выносят? А ведь метод то в 3 строки, т.е. делает чтото простое.

Функция, принимающая в качестве параметра функцию, принимающую в качестве параметра ещё одну функцию, по большому счёту, не сложнее трёх вложенных циклов или массива с тремя индексами.

_>2. clipboardDataUpdater — вызывается в конце кода! Проще чтобы SetData вернул что надо, а делегат вызвать следом.

clipboardDataUpdater(data, () => Clipboard.SetDataObject(data, true));

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

Здравствуйте, Sharowarsheg, Вы писали:

S>Здравствуйте, barn_czn, Вы писали:

S>>>Может быть. Попробуй объяснить, что тебе кажется тут не так?

_>>1. Action<DataObject, Action> — вот такие делегаты, принимающие делегата вам мозг не выносят? А ведь метод то в 3 строки, т.е. делает чтото простое.

S>Функция, принимающая в качестве параметра функцию, принимающую в качестве параметра ещё одну функцию, по большому счёту, не сложнее трёх вложенных циклов или массива с тремя индексами.

Да нет батенька, и тройные циклы встречаются чаще, и быдлокодерам функцию принимающая функцию куда сложнее понять чем тройной цикл.

_>>2. clipboardDataUpdater — вызывается в конце кода! Проще чтобы SetData вернул что надо, а делегат вызвать следом.

S>

S>clipboardDataUpdater(data, () => Clipboard.SetDataObject(data, true));
S>

S>Ну да, можно на две строки переписать, читать будет легче, но это отчасти зависит от того, как у вас заведено. Я бы переписал, но это не ужас-ужас-ужас.

Да дело ни в ужас. А "на..уя??" Вы в дотнете много видели делегатов принимающих делегат? Я не припомню. А тут ради 3х строк, и делегат в делегата.. помилуйте мозг тех кто это должен юзать.

Здравствуйте, barn_czn, Вы писали:

S>>Функция, принимающая в качестве параметра функцию, принимающую в качестве параметра ещё одну функцию, по большому счёту, не сложнее трёх вложенных циклов или массива с тремя индексами.

_>Да нет батенька, и тройные циклы встречаются чаще, и быдлокодерам функцию принимающая функцию куда сложнее понять чем тройной цикл.

Смотря какие циклы, смотря как у вас принято, и смотря на сколько быдлокодеры.

S>>

S>>clipboardDataUpdater(data, () => Clipboard.SetDataObject(data, true));
S>>

S>>Ну да, можно на две строки переписать, читать будет легче, но это отчасти зависит от того, как у вас заведено. Я бы переписал, но это не ужас-ужас-ужас.

_>Да дело ни в ужас. А "на..уя??" Вы в дотнете много видели делегатов принимающих делегат? Я не припомню. А тут ради 3х строк, и делегат в делегата.. помилуйте мозг тех кто это должен юзать.

Ну, напишите стандарт, типа делегаты принимающие делегатов запрещены, по соображениям снижения сложности. Чего ради оуевать-то? Ну бывает, напишется что-то забавное. Я и веселее делал в своё время, да и ещё сделаю наверняка, так что ничего особенно выдающегося.

Это же не ради трёх строк, это ради гибкости или чего-то типа такого, скорее всего (насколько я могу влезть в голову автору).

Здравствуйте, barn_czn, Вы писали:

S>>Может быть. Попробуй объяснить, что тебе кажется тут не так?

_>1. Action<DataObject, Action> — вот такие делегаты, принимающие делегата вам мозг не выносят? А ведь метод то в 3 строки, т.е. делает чтото простое.

Нет, не выносят.

_>2. clipboardDataUpdater — вызывается в конце кода! Проще чтобы SetData вернул что надо, а делегат вызвать следом.

IMHO, нормально сделано, "в функциональном стиле", гибко.

Например, clipboardDataUpdater сможет внутри решить, в какай момент вызывать дефолтный метод Clipboard.SetDataObject(data, true) (читай — инициализация и очистка),
и вызывать ли вообще (читай — юнит-тесты), а то может лучше гранату бросить отформатировать диск вместо этого.

Нужна ли такая свобода действий? Хз, зависит от проекта. В обычном случае — нет. Но если это библиотека,
то чем больше гибкости — тем больше вероятность что она окажется полезной, и что ее будут использовать.

Мне этот код не нравится прежде всего тем, что глядя на контракт функции

void SetData(Action<DataObject, Action> clipboardDataUpdater);

совершенно неясно, как этим правильно пользоваться.

Ведь как будет рассуждать программист, которому отдадут такую библиотеку?
Ну ОК, надо передать внутрь функцию, принимающую два параметра и можно ожидать, что в какой-то момент эта функция будет вызвана инфраструктурным кодом.
Первым параметром мы получим DataObject. Наверное, это объект для сохранения в буфер. Ладно.
Вторым параметром будет какой-то Action. Ээээ... и чего нам с ним делать-то?

Если немного переписать этот код:

/// <summary>
/// An action to be called once clipboard data are ready to be saved to the clipboard
/// </summary>
public delegate void CommitAction();

/// <summary>
/// Clipboard data updater callback
/// </summary>
/// <param name="clipboardData">Clipboard data to be customized by the caller</param>
/// <param name="commit">Commit action which must be invoked by the caller to save the data to the clipboard</param>
public delegate void ClipboardDataUpdaterCallback(DataObject clipboardData, CommitAction commit);

public static void SetData([NotNull] ClipboardDataUpdaterCallback clipboardDataUpdater)
{
    Clipboard.Clear();
    var data = new DataObject();
    clipboardDataUpdater(data, () => Clipboard.SetDataObject(data, true));
}

то контракт уже становится более явным.

И дальше остаются только вопросы к дизайну такого решения.
Чего хотел добиться автор?
Я могу предполагать следующее:
1. Скрыть от пользователя библиотеки детали работы с буфером обмена.
Имхо, это не особо удалось. Т.к. пользователю все равно придется работать с Windows-специфичным API в DataObject.
2. Дать возможность пользователю решать, будут ли изменения записаны в буфер
Мне лично как-то сложно представить ситуацию, когда я заполнил DataObject данным и вдруг решил его не сохранять.
Если на вашем проекте такое случается — то ОК, подход оправдан.
3. Дать пользователю самостоятельно определять момент, когда изменения будут зафиксированы в буфере
Для меня типичный use-case такой: получить откуда-то данные, заполнить DataObject и закомитить (или не закомитить) изменения.
Выдумать какой-то еще полезный вариант у меня не получилось.
4. Наконец, если сравнивать

// (1)
SetData((data, commit) =>
{
    // Update data here
    commit();
});

// (2)
Clipboard.Clear();
var data = new DataObject();
// Update data here
Clipboard.SetDataObject(data, true);

то, второй вариант всяко понятней первого, и не на много длиннее.

Здравствуйте, barn_czn, Вы писали:

_>1. Action<DataObject, Action> — вот такие делегаты, принимающие делегата вам мозг не выносят?

Нет.

_>2. clipboardDataUpdater — вызывается в конце кода! Проще чтобы SetData вернул что надо, а делегат вызвать следом.

При этом потеряется ленивость, это может быть важно.

Здравствуйте, RushDevion, Вы писали:


RD>Мне лично как-то сложно представить ситуацию, когда я заполнил DataObject данным и вдруг решил его не сохранять.

Ну с буфером МНЕ тоже не нужно было такое, но например с xml — такое бывает. То есть создал xml element, "типа" заполнил, а потом понял, что сохранять то его и не нужно. Наверняка и с буфером такое случается.

Здравствуйте, barn_czn, Вы писали:

_>Хелп! Объясните мне люди, я чего то не понимаю в этом мире.

_>Код ниже я считаю откровенным гвнокодом. Не могу никак доказать коллеге что это так.

Вполне гибко сделано, да читается чуть хуже.

С помощью него вы можете , если это будет необходимо, легко расширить поведение, например логирование , дополнительные проверки.
Также можно написать другой алгоритм SetData, например для другой платформы, не меняя логику Updater

void ClipboardUpdater( DataObject obj,  Action setClipboardData)
{
   try
   {
       setClipboardData?.Invoke();
   }
   catch(Exception e)
   {
       log(e);
       throw;
   }
}


void ClipboardUpdater( DataObject obj,  Action setClipboardData)
{
       if (DataObject ... )
          setClipboardData?.Invoke();
}

_>Да не, ну этот метод считают крайне полезным, его в библиотеку тянут ).
_>Хрен с ним коллегой, мне за такую библиотеку стыдно.

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