- 01
- 02
- 03
- 04
- 05
- 06
- 07
- 08
- 09
- 10
- 11
- 12
- 13
- 14
- 15
- 16
- 17
- 18
- 19
- 20
- 21
- 22
- 23
- 24
char *uart_readln_timeout(UART_HANDLE h, char *buf, uint16_t len, clock_tick_t to)
{
clock_tick_t finish_time;
char *datap = buf;
char *datae = buf + len - 1;
if( len == 0 ) return buf;
if( len == 1 ) {
buf[0] = 0;
return buf;
}
finish_time = clock_get_millis() + to;
// FIXME!!!
while( datap < datae && ( to == 0 || clock_get_millis() < finish_time ) ) {
if( uart_read_char(h, datap) ) {
if( *datap++ == '\n' ) break;
}
}
*datap = 0;
return buf;
}
читает строку из UART. есть подозрение, что это говнокод.
Follow us!
Хотя... контроля длины строки нет...
================================
Заметил вот что вызов clock_get_millis() в while, хотя судя по названию функции, быть может так и надо.
Так вроде ничего не заметил, минусую.
1) В условных выражениях константы должны быть слева (тут же: to == 0 и т.д.).
2) манипуляции с datap, datae и создание finish_time происходят слишком рано. Должны быть после первых двух if'ов, а то напрасная трата ресурсов.
3) datap, datae - ваще говорящие имена.
4) buf не проверяется на 0, а надо бы.
5) if( *datap++ == '\n' ) break; - экономисты, блин, строк кода. Нечитаемая конструкция.
Нормально так заметил, плюсую.
1) Это чисто ваши личные причуды. На сгенерированный код никак не влияет.
2) Насчет datap и datae согласен, они инициализируются слишком рано. А про finish_time не забывайте, что это C, а не C++. К тому же, если все переменные создаются в начале функции, то место в стеке выделяется, грубо говоря, одной инструкцией, а если в разных местах - то уже несколькими.
3) Нормальные имена, опять же, это ваши личные причуды.
4) Проверять указатель на 0? В embedded? Уже смешно. Ну допустим, а что вы сделаете, если он таки ноль?
5) Нормальная компактная конструкция, а вы учитесь читать.
а) в С/С++ что бы не было if (a=0)
б) в Java/C# что бы не было NPE при if (a.eqals("QQQ")) (правильно: if ("QQQ".eqals(a))
в) выражения вида if (someFunctionName(operandName + opN ) == 0 ) не всегда легко читаются из-за своей длины, а если константа слева её сразу видно.
2) finish_time - да, забыл, что С, а не ++. Хотя под солярисом gcc и такое в "сишнном" коде съедал.
3) имя переменной должно говорить о том, чем она является. Что такое datap? Это даже не слово английского языка.
4) Программисты embedded не делают ошибок? Уже смешно. Если думаете, что от if упадёт производительность, то поставьте assert и пусть компилятор вырежет его в "релизе".
5) Да! Даёшь код всей программы в одну строку :D
Поправка!
в Java, но не в C#, где достаточно a == "QQQ"
1а) как бы защищает, но нигде чёта не видел такого.
2) да и хуй с этим с finish_time
3) автор кода лентяй ))
4) ещё как делают, ток выявлять их дольше. if( uart_read_char(h, datap) ) нихуя не понял зачем, буффер параметром ведь передается
5) Нииееет, всё ок с такой конструкцией. Это чтоб шпиону сложней было разбираться ;)
видимо, так: if(0 == a - b)
но для строк не катит...
1. Слово "должны" тут неуместно. Крайне полезный совет для начинающих писать на С, С++ - ставьте константы справа, это перевалит на компилятор проверку конструкций вида if (to = 0). Однако через три месяца программирования такие конструкции цепляются взглядом и без помощи компилятора. Это не доктрина, это совет.
2. Согласен, чем меньше время жизни переменной, тем легче читать код
3. Не только эти. Вообще имена в коде говеные. Не буду голословным: что такое len? Длина принятых данных? Не, ошибка, это размер буфера. Что такое buf? Буфер? Опять ошибка - выходная С-строка. Хуже нечитаемых имен переменных, только имена переменных вводящие читателя в заблуждение.
4. Все верно, при передаче в функцию указателя, ответственность по проверке актуальности указателя лежит на функции. Если эта проверка может быть сделана, она должна быть сделана.
5. Действительно, плохо читаемая конструкция. В основном засчет изменения переменной в области проверки оператора if. Почти так же плохо как if (to = 0) ...
от себя добавлю:
6. Тут много говорили о встроенной платформе и оптимизации кода. Но тогда проверка to==0 в while не лезет ни в какие ворота: если ждать до времени - она лишняя.
7. Возвращаемое функцией значение - лишнее: buf не модифицируется и возвращается всегда.
8. Проверка len == 1 - выделение частного случая. Без нее вполне можно обойтись.
Плюсую и говнокод и замечания.
Это позволило бы исключить одну проверку из цикла.
Хотя, ИМХО, обращать внимание на такие детали во время написания кода - большое зло. Оптимизировать код нужно тогда и только тогда, когда доказана необходимость оптимизации и найдено узкое место.
1) Схуя константы слева?? Таково нигде нет, иф (0 == т) - хуита, Номад отдыхает. Это на троллинг похоже.
2) насчёт datap и datae как бы формально вы правы. Но что, если я хочу отдебажить и всегда видеть, что в буфере. При оптимицации, думаю лишние ресурсы не уйдут.
3) datap - data_pos, datae - data_end, что непонятного?
4) Нафиг буф на 0 проверять, если есть len, который увеличивается в обработчике прерывания уарта. После раздрочки буффера или таймауте обнуляеся len в оддельной функции, и не надо тратить ресурсы на обнуление всего буффера.
5) В принципе да, на первый взгляд нечитаемо. Но когда в следующий раз видишь (*some_ptr++ == some_const) сразу становится понятно.
Вроде не говнокод. Минусовать нада? Я тут новенький.
Вообще, если человек пишет так, как понятно ему, но не понятно вам, это не значит, что человек не прав. Самое главное в коде - это результирующий код после компиляции (безглючный, быстрый и т.п.) и возможность поддерживать. Лично я не вижу в данном стиле написания ничего такого, что например мне мешало бы поддерживать данный код, если бы он перешёл мне. Да есть непривычные на глаз вещи, как, например, эти "(len == 0)", ибо я привык писать "(!len)"; но это лишь из-за того что я редко встречаю "(len==0)", а не потому что это является уродством.
Вот к примеру, вам не нравятся названия переменных "buf" и "len", а ведь лично я достаточно точно понимаю их смысл в коде, если вы нет - вероятно у нас с вами просто разное восприятие. "buf" - это буффер, в нём есть какие-то данные, которые в следствии как-то обрабатывались, и к концу работы функции, так получилось, что стали результирующей строкой. Зачем вводить ещё один поинтер указывающий туда же называя её "result"? Лично я как вижу функцию, понимаю, что buf - это просто указатель на начало буффера, а что в это буффере храниться этой переменной никак не касается, ибо повторюсь, buf - это просто указатель на начало какого-то буффера. А len - это опять очень очевидная по её применению переменная, это просто дополнительная переменная характеризующая состояние переменной "buf", этому len тоже пофиг как используют данные зарытые в данном буффере, у этой переменной основная функция указывать лишь на длинну буффера и всё. Дак вот после этих и так очевидных тезисов идём дальше, названия переменных вполне нормальные, новые создавать смысла нет, а самое оптимальное - итогую строку реализовать прямо в этом же буффере, отсюда и "return buf;". Я вот вообще не вижу никакой проблемы в данном названии переменной buf.
Вот например, я и кто-нибудь из вас пишем в разных стилистиках. Допустим оба пишем хороший безглючный, шустрый код, в которых оба сможем легко разобраться спустя несколько лет. Не будем же мы считать код друг друга - говнокодом? Это бред. Ибо если считать в данной ситуации код друг друга - говнокодом, то на любой код найдётся человек, для которого этот код будет говнокодом.