Что то часто мне стал попадаться код в который сразу не врубишься, при этом код делает свою задачу правильно. Предлагаю постить сюда ваши образчики и комментарии.
public void WriteTransformation(ref byte[] buffer, ref int bufferLength, WriteByteHandler writeByteHandler)
{
if (writeByteHandler != null)
{
bufferLength = 1 + Math.Min(Math.Max(bufferLength, 0), (buffer != null) ? buffer.Length : 0);
byte[] newBuffer;
if (bufferLength - 1 < ((buffer != null) ? buffer.Length : 0))
{
newBuffer = buffer;
}
else
{
newBuffer = new byte[bufferLength];
}
for (int i = bufferLength - 1; i > 0; i--)
{
if (newBuffer != null) if (buffer != null)
newBuffer[i] = buffer[i - 1];
}
if (newBuffer != null) newBuffer[0] = PREFIX;
buffer = newBuffer;
}
}
Самое смешное в том, что часть строки я прочел как
Здравствуйте, AlexNek, Вы писали:
AN>Что то часто мне стал попадаться код в который сразу не врубишься, при этом код делает свою задачу правильно. Предлагаю постить сюда ваши образчики и комментарии.
А CopyTo + запись префикса — слишком просто?
За ref-ы, допущение того, что bufferLength может быть меньше 0, непроверку на переполнение — приговорить к пожизненному сопровождению своего кода.
Здравствуйте, AlexNek, Вы писали:
AN>Что то часто мне стал попадаться код в который сразу не врубишься, при этом код делает свою задачу правильно. Предлагаю постить сюда ваши образчики и комментарии.
AN>>Что то часто мне стал попадаться код в который сразу не врубишься, при этом код делает свою задачу правильно. Предлагаю постить сюда ваши образчики и комментарии.
Мде. Поспешил запостить.
Неужели вы занимаетесь ревью чужого кода?
В целом написано, конечно, топорно. Человек не знал что есть такая штука как "??", но код будет работать. Единственное, что умиляет — newBuffer = buffer. Вот тут как раз надо человека засадить за изучение описания C# из MSDN и конкретно книгу Джеффри Рихтера.
AN>Незнаю как вы, но большой одной строки для foreach я как то слабо представляю, поэтому также не смог сразу врубиться, не говоря уж об обилии лямбд
Вот тут как раз надо смотреть как работает механика на лямбда. Иначе у меня такое подозрение, что запрос энтот будет выполнятся на каждой итерации цикла. Возможно даже цикл получится бесконечным. Ну и помаркать сообщения можно бы было напрямую лямбда-запросом. Но это уже всякие извращения.
Возможно выглядит проще. Но при изменении типа IsRead, к примеру было bool, стало bool?, придётся добавить вызов FindAndMarkMsgs(null). В варианте же с группировками ничего менять не надо.
Здравствуйте, Andrey Rubayko, Вы писали:
AR>…Человек не знал что есть такая штука как "??", но код будет работать.
А где бы вы применили этот оператор в показанном коде?
AR>Единственное, что умиляет — newBuffer = buffer. Вот тут как раз надо человека засадить за изучение описания C# из MSDN и конкретно книгу Джеффри Рихтера.
А не расскажете поподробнее?
Help will always be given at Hogwarts to those who ask for it.
Здравствуйте, _FRED_, Вы писали:
_FR>Здравствуйте, Andrey Rubayko, Вы писали:
AR>>…Человек не знал что есть такая штука как "??", но код будет работать.
_FR>А где бы вы применили этот оператор в показанном коде?
Ааа. Недосмотрел.
Там таки длину присваиваем
Просто давече видел другой код:
(buffer != null) ? buffer : 0
Вот и подумал что тоже самое.
AR>>Единственное, что умиляет — newBuffer = buffer. Вот тут как раз надо человека засадить за изучение описания C# из MSDN и конкретно книгу Джеффри Рихтера.
_FR>А не расскажете поподробнее?
Ну как. В данном случае просто копируется указатель на массив buffer в newBuffer. Честно говоря, мож так и надо и мы не будем использовать начальное значение переменной buffer. Так что ладно.
Здравствуйте, AlexNek, Вы писали:
AN>Что то часто мне стал попадаться код в который сразу не врубишься, при этом код делает свою задачу правильно. Предлагаю постить сюда ваши образчики и комментарии.
[skiped]
AN>Интересно сколько времени вам понадобилось, что бы понять как функция точно работает?
Более того, я осмелился этот код переписать. Вот что у меня вышло:
public byte[] WriteTransformation(byte[] buffer, int bufferLength)
{
// Выделяем проверку предусловий в отдельный блок, что на порядок упрощает весь остальной код.
// Контракты просто упрощают запись, но никто не мешает проверить предусловия руками.
Contract.Requires(buffer != null);
Contract.Requires(bufferLength >= 0);
Contract.Ensures(Contract.Result<byte[]>() != null);
// Прежде всего убираем не используемый параметр, если же он используется, то добавляем
// соответствующее предусловие.
// А теперь демонстрируем уличную магию того, как проверка предусловий упрощает код:
// Было: bufferLength = 1 + Math.Min(Math.Max(bufferLength, 0), (buffer != null) ? buffer.Length : 0);
// Меняем: (buffer != null) ? buffer.Length : 0 --> buffer.Length, благодаря buffer != null
// Меняем: Math.Max(bufferLength, 0) --> bufferLength, благодаря bufferLength >= 0
// Добавляем комментарий и вот, что мы получаем в итоге:
// Необходимость +1 обусловлена тем, что в нулевой байт выходного буфера мы поместим PREFIX
bufferLength = 1 + Math.Min(bufferLength, buffer.Length);
byte[] newBuffer;
// Теперь меняем следующий код:
// Было: if (bufferLength - 1 < ((buffer != null) ? buffer.Length : 0))
// Проверка buffer != null, уже не нужна
// Получаем: if (buffer.Length > bufferLength - 1)
// Заменяем > на >=:
// if (buffer.Lengh >= bufferLength)
// Инвертируем логику, поскольку она проще
// Результат: if (buffer.Lenfth < bufferLenfth) ...
// Добавляем комментарий и вот, что получаем в результате:
// Нам нужно удостовериться, что в исходном массиве есть место под дополнительный
// байт префикса:if (buffer.Length < bufferLength )
{
newBuffer = new byte[bufferLength];
}
else
{
newBuffer = buffer;
}
// Все, что удалось улучшить в этом коде, так это избавиться от лишних
// проверок и добавить комментарий, поясняющий причину перебора элементов
// "справа". Кстати, Array.CopyTo здесь не подойдет, поскольку он копирует
// *все* элементы массива, а реальный размер массива buffer может быть большим
// Добавляем комментарий:
// Теперь нам нужно скопировать все элементы из массива buffer в newBuffer, сместив
// каждый из них на один индекс вправо, чтобы в нулевой индекс поместить PREFIXfor (int i = bufferLength - 1; i > 0; i--)
{
newBuffer[i] = buffer[i - 1];
}
// Здесь тоже проверка уже не нужна
newBuffer[0] = PREFIX;
// У нас все еще есть проблема, поскольку в некоторых случаях мы меняем переданный буфер,
// так что, если размер этого массива обычно небольшой, то я бы предпочел, чтобы всегда
// возвращался новый объект.
// Неизменяемость (или имутабельность) - это отличная техника программирования!return newBuffer;
}
Здравствуйте, SergeyT., Вы писали:
ST> AN>Что то часто мне стал попадаться код в который сразу не врубишься, при этом код делает свою задачу правильно. Предлагаю постить сюда ваши образчики и комментарии.
ST> [skiped]
ST> AN>Интересно сколько времени вам понадобилось, что бы понять как функция точно работает?
ST> Более того, я осмелился этот код переписать. Вот что у меня вышло:
Здравствуйте, SergeyT., Вы писали:
ST> Вчера наткнулся на следующий код разбора аргументов командной строки (код упрощен, но сам цикл и разбор аргументов сохранен):
ST> С кодом все нормально, но чтобы понять, нет ли ошибки на единицу нужно каждый раз напрягаться.
Чтобы понять как работает также нужно напрягаться.
Называется "экономия строк"
U> AN>Незнаю как вы, но большой одной строки для foreach я как то слабо представляю, поэтому также не смог сразу врубиться, не говоря уж об обилии лямбд
U> По-моему как раз наоборот простой читаемый код
Может дело привычки, но первое что я увидел, было
foreach (var group in hash
Затем уже начинаешь соображать, а нахрена остальные строки. Даже такой записи уже было бы достаточно
foreach (var group in hash
.GroupBy(rq => rq.IsRead)
.Select(grp => new { IsRead = grp.Key, Ids = grp.SelectMany(rq => rq.MsgIds) }))
{
MarkMsgsRead(
_provider,
group.Ids,
group.IsRead);
}
Кстати, не поясните, что делает эта часть (какой тип возвращает return)? (У нас в проекте linq не используют)
var groups = groupBy.Select(
delegate(IGrouping<bool, MarkRequest> grp)
{
return new {IsRead = grp.Key, Ids = grp.SelectMany(rq => rq.MsgIds)};
}
);
AR> AN>Незнаю как вы, но большой одной строки для foreach я как то слабо представляю, поэтому также не смог сразу врубиться, не говоря уж об обилии лямбд
AR> Вот тут как раз надо смотреть как работает механика на лямбда. Иначе у меня такое подозрение, что запрос энтот будет выполнятся на каждой итерации цикла. Возможно даже цикл получится бесконечным. Ну и помаркать сообщения можно бы было напрямую лямбда-запросом. Но это уже всякие извращения.
Что то у меня есть такое подозрение что foreach вызывает "enumerable-expression" только один раз.
Здравствуйте, Sinix, Вы писали:
S> AN>Что то часто мне стал попадаться код в который сразу не врубишься, при этом код делает свою задачу правильно. Предлагаю постить сюда ваши образчики и комментарии.
S> А CopyTo + запись префикса — слишком просто?
Вопрос не ко мне . Если есть желание, можете просто выложить Вашу версию.
S> За ref-ы,
У функции уже были параметры (без рефов), менять слишком много было просто запрещено. S> допущение того, что bufferLength может быть меньше 0,
Пути господни неисповедимы
x> Порадовало такое ощущение что код писал параноик)
Эээ... батенька вас надо с нашей QA группой познакомить x> Да и на фига массив по ссылке передавать?)))
Я же говорю код тяжело понять Входной массив не обязательно будет "константой".
x> А можно ли постановку задачи, чтобы выложить свою версию метода?
Но проблемо. Кстати, эта функция раньше была гораздо проще.
Есть некоторый массив байт, который нужно в итоге переслать в выходной поток. Но перед этим нужно в начале добавить один байт.
Примечание:
Добавить байт в буфер вначале, до данных,
непонятно как, так как при другом типе выходного потока байт добавлять не нужно,
а данные приходят "снаружи".
Раньше, копировать данные абсолютно не хотелось и дополнительный байт просто выводился прямо в поток, а после все остальное. Однако оказалось, что минимум под виндовс 7 такой способ передачи резко не нравится приемнику он требовал передавать все сразу.
Кстати размер буфера не может быть больше 256 байт и в реале получается от 10 до 150 байт.
Здравствуйте, AlexNek, Вы писали:
AN> ST> AN>Что то часто мне стал попадаться код в который сразу не врубишься, при этом код делает свою задачу правильно. Предлагаю постить сюда ваши образчики и комментарии.
AN> ST> [skiped]
AN> ST> AN>Интересно сколько времени вам понадобилось, что бы понять как функция точно работает?
AN> ST> Более того, я осмелился этот код переписать. Вот что у меня вышло:
AN> А для NET 2.0 это тоже будет работать?
Будет, если либу загрузить...
А если не нужны "чужие сообщения" либо вообще какие либо сообщения?