LINUX.ORG.RU

Отрецензируйте, пожалуйста, код

 ,


1

1

Как кто-то говорил на ЛОР-е, настоящий программист должен сделать 3 вещи:

1) Написать helloworld
2) Написать калькулятор
3) Написать аудиоплеер

Собственно, первые 2 пункта уже имеются. А тема это о калькуляторе. Писался он ради развлечения и повышения собственных навыков GUIстроения. Ссылка: https://github.com/xdevelnet/n0calc

Язык: c99 (с возможность легко переписать на c89)
Тулкит: GTK3

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

Дополнительно хочу поблагодарить хороших и добрых людей, которые мне помогали в прошлой теме и остальным людям, которые наставляли меня на путь истинный :) Низкий поклон вам!

P.S. «поревьюйте» - я правильно написал?

★★★★★

Последнее исправление: reprimand (всего исправлений: 2)

Ответ на: комментарий от anonymous

Надеюсь ты не потратил на это больше 10 минут?

изучение тулкита? ну я даже не знаю, Карл

reprimand ★★★★★
() автор топика
Ответ на: комментарий от i-rinat

Calculator evaluates expression at specified format: operand1*operand2

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

reprimand ★★★★★
() автор топика
Последнее исправление: reprimand (всего исправлений: 1)
Ответ на: комментарий от anonymous

У него разница между коммитами порядка 10 часов.

Deleted
()
Ответ на: комментарий от reprimand

1) нет автомата состояний

2) нет нормальной работы с числами при переполнении, например делим большое число на околунулевое

садись, 3 с большим минусом...

я когда то писал калькулятор с поддержкой систем счисления от двоичной до шестнацетиричной. (13чная, например, тоже поддерживалась). вот это был калькулятор, проект из этак десятка классов, где четкое разделение. У тебя же получается все в кучу смешано, если начинать расширять, то уже будет плохо.

fornlr ★★★★★
()
Последнее исправление: fornlr (всего исправлений: 1)
Ответ на: комментарий от four_str_sam

Например смешиваешь интерфейс и обработку данных.

если бы я писал вменяемый парсер выражения (со скобками, степенью, etc), естественно, отделил бы. А так - просто не видел смысла.

reprimand ★★★★★
() автор топика
Ответ на: комментарий от fornlr

1) нет автомата состояний

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

2) нет нормальной работы с числами при переполнении, например делим большое число на околунулевое

It was created just for fun and improving my GUI creation skills.

садись, 3 с большим минусом...

прости, но тебе не удастся меня унизить :)

я когда то писал калькулятор с поддержкой систем счисления от двоичной до шестнацетиричной. (13чная, например, тоже поддерживалась). вот это был калькулятор, проект из этак десятка классов, где четкое разделение.

Ссылку на проект можно, пожалуйста?

У тебя же получается все в кучу смешано, если начинать расширять, то уже будет плохо

тут есть только одно место с «в кучу смешано» - это validate_and_calc(). Зачем я смешал уже ответил в теме.

reprimand ★★★★★
() автор топика
Ответ на: комментарий от fornlr

для школьника, меня за такое били бы томом Кнута в университете

за отсутствие автомата состояний, объединение 2-х функций в 1 из-за простоты программы? или за соответствие описанию программы самой программе?

пожалуйста, если у тебя есть еще замечания, сообщи

Я реально хочу выпрямить руки. Правда-правда.

А профильного образования у меня нет, от слова совсем.

reprimand ★★★★★
() автор топика
Ответ на: комментарий от anonymous

Мы арифметическую парсилку (читай — калькулятор без гуя) на первом курсе писали. Для студента эта вот бестолковая формочка вообще не вполне.

staseg ★★★★★
()
Ответ на: комментарий от itn

А почету не c11?

Что именно из С11 глубокоуважаемый специалист рекомендовал бы использовать?

anonymous
()
Ответ на: комментарий от i_gnatenko_brain

если ты используешь Gtk - используй GObject

спасибо, буду смотреть что оно еще может

в конкретно этом случае просто не видел смысла использовать gchar:
https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gchar

ну и как-то классические типы привычнее что-ли

reprimand ★★★★★
() автор топика
Ответ на: комментарий от theNamelessOne

А смысл делать на отъебись?

Пожалуйста, прочитай описание.

Плюс я не нашел смысла сделать полноценный калькулятор из-за наличия имеющегося mate-calc.

Ты же не лабораторную сдаешь.

у меня их и не было никогда

reprimand ★★★★★
() автор топика
Последнее исправление: reprimand (всего исправлений: 1)

Что-то в последнее время часто подобные треды появляться стали. Тоже что-ли свою поделку выставить?

Kilte ★★★★★
()
Ответ на: комментарий от reprimand

О, спасибо, что вы оценили мое предложение
Я вот на гитхабе нашел секундомер на Гтк3 и на headerbar перевожу потихонечку
Удачи тебе, альтернатива стандартным приложениям из Гнома должны быть, а то какая-то опенсорсная монополия )

mystery ★★
()
Ответ на: комментарий от Freyr69

Полиморфизм?

_Generic, который? Он годится разве что для костыльной эмуляции перегрузки функций. Для интерфейсов к библиотекам это еще может быть полезным, но ТС же не библиотеку писал.

anonymous
()
Ответ на: комментарий от mystery

Удачи тебе, альтернатива стандартным приложениям из Гнома должны быть, а то какая-то опенсорсная монополия )

Спасибо =)

ну, я не особо планировал писать замену стандартным приложениям, которые есть в поставке DE

а вот парочка программ, которых практически нет (точнее они есть, но либо уродливы либо не-standalone) у меня есть на примете...

reprimand ★★★★★
() автор топика

Еще у тебя баг при вычитании, и в целом говнопарсинг. Зачем там вообще точки искать? Ешь число strtod'ом, ассерт на blank/op, ешь второе число.

arturpub ★★
()

counting the specified character from the string
replaces all characters from specified one to specified other one
much dots
much operators
expression at
called as
reason to

Не говоря уже о полном отсутствии артиклей.
Я бы никогда не стал пользоваться программой с комментариями на таком «английском».

Lincor
()
Ответ на: комментарий от Lincor

Системдой же пользуются. У Лени с английским тоже вроде херовенько. (Но в душе лорчую.)

arturpub ★★
()

Код не смотрел но осуждаю

expelled ★★
()
Ответ на: комментарий от arturpub

char[31]/memset/snprintf используй sizeof

точно! благодарю, странно что я раньше не обратил на это внимание

не надо вычитать 1 для snprintf

нуль терминатор?

Еще у тебя баг при вычитании

ошибся знаком. Исправлено.

и в целом говнопарсинг

уже писал в треде о нём

Зачем там вообще точки искать? Ешь число strtod'ом

Чтобы делать парсинг числа с плавающей точкой вручную. Странно, когда искал подходящую функцию, вчера натыкался на какие-то варианты, доступные с c99, а strtod() почему-то либо провтыкал либо забраковал. Спасибо за замечание! Наверное, ночью что-то делать с заплывшими глазами мне пора прекращать...

reprimand ★★★★★
() автор топика
Ответ на: комментарий от Lincor

ну тогда от принятого императивного стиля написания commit месседжей у тебя бы, видимо, совсем полыхнуло

допускаю, что в комментариях есть ошибки, ибо помню, что проверял только лишь readme.md, комментарии не пересматривал

Я бы никогда не стал пользоваться программой с комментариями на таком «английском».

на вкус и цвет...

reprimand ★★★★★
() автор топика
Ответ на: комментарий от Deleted

Лол

выше по теме уже сказали

бывает =)

reprimand ★★★★★
() автор топика
Ответ на: комментарий от mystery

Рад буду помочь

не нашел контактов в профиле

Есть же люди хорошие, делающий линукс лучше и лучше!

благодарю, буду стараться!

reprimand ★★★★★
() автор топика
Ответ на: комментарий от reprimand

объединение 2-х функций в 1 из-за простоты программы

Это не KISS и не имеет вменяемых оправданий, кроме лени разработчика, так что бить томиком Кнута по рукам надо. А вообще кода там очень мало, чтобы хоть какую-то адекватную и полноценную критику кроме этой писать, не в даваясь в детали технические, а не системные.

peregrine ★★★★★
()

В заголовочных файлах переменные не определяют, и вообще непонятно зачем этот файл. Непонятная пляска с nmbrs/numbers. Вместо дефайнов подошел бы enum.

Если функция не меняет строку - делай const char *. while идиоматически переписывается на for.

Не надо в одну функцию мешать GUI и вычисление, даже в хелловорлде.

button_exec - сплошной копипаст, можно без него. place_and_bind_button - тоже, лучше меняющиеся данные занести в массив структур и сделать по нему цикл. Или похитрее логику найти, чтоб не ломать голову в заборе «3, 0, 1, 1».

static double result - не очень уместный static. Лучше результат хранить вне функции, и передавать если надо.

Магические числа: 24, 30, 31. Используй sizeof, например. {«Malformed expression at 1 operator!»} - скобки лишние.

Некоторые функции static, некоторые нет.

Отрицательные индексы массива - нарушение стандарта.

Разбор операндов копипастится. sscanf скорее всего хватило бы.

anonymous
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.