Вопросы по С/С++ (СИ)

ra9ust

Понимаю. Тогда как сделать сравнение всех тех переменных находящихся в ОЗУ с их значениями в EEPROM ?

Добавлено after 2 minutes 10 seconds:
Согласен, в куче. Но я свободно ориентируюсь , возможно потому что сам составлял. По той же причине мне непонятен любой чужой код.Ну так как насчет сравнения переменных в епром?

Реклама

[uquote="ra9ust",url="/forum/viewtopic.php?p=3219974#p3219974"]как сделать сравнение всех тех переменных находящихся в ОЗУ с их значениями в EEPROM ?[/uquote] Структуры + побайтовое сравнение через указатель.

Реклама

ra9ust

А покажите это на примере,упрощенно?

arkhnchul

код ну совсем ужасен, честно. Отмазка "я в нем разбираюсь" не принимается.
1) форматирование. Отступ - три или четыре пробела, а не один.
2) магические числа везде. Не должно такого быть, менять на константы или дефайны с осмысленными именами.
3) из названия функции должно быть понятно, что она делать. Вот что за temperature()?
4) откройте для себя switch
5) раз пишете сообщения и идентификаторы по английски и знаете его плохо, пользуйтесь переводчиком. Хотя бы для сообщений.
6) огромные цепочки условий в if-ах сокращаем и упрощаем. Думаем, как. В самом крайнем случае, если с ними вообще сделать ничего нельзя (что обычно не так) - выделяем скобками логически связанные блоки
7) не стесняемся использовать массивы и структуры вместо кучи переменных с индексами. Это к мешанине hour_al_1 и иже с ним.

избавляйтесь от кучи одинаковых функций, все отличие которых - разные переменные. Сему способствует пункт 7.

Реклама

ra9ust

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

**избавляйтесь от кучи одинаковых функций, все отличие которых - разные переменные. Сему способствует пункт 7.** и с этим согласен, но подскажите как? На простом примере.

Реклама

[uquote="ra9ust",url="/forum/viewtopic.php?p=3219982#p3219982"]А покажите это на примере,упрощенно?[/uquote] Пример чего интересует ? Организацию структуры, или работу с указателем ?

Реклама

ra9ust

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

ra9ust писал(а):куча сравнений переменных с их значениями в еепром, как записать это компактней?

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

PS: Надеюсь, что такое структуры и указатели не нужно рассказывать ? Если что, всё гуглится без проблем.

arkhnchul

[uquote="ra9ust",url="/forum/viewtopic.php?p=3220033#p3220033"]Так, по магическим числам поясните,непонятно.[/uquote]

Код: Выделить всё

...
if(cc>=254)cc=14;
...
if(w==72)w=60;
if(w==93)w=90; 
...
if(eeppp&0b0111111110010000)
...
lcd_putchar(0x42)
...

что такое 14? 72? 60? 93? 90? 0b0111111110010000? 0x42? Это и есть magic numbers - какие-то числа, что-то там означающие, но неизвестно что. Они все должны быть константами, дефайнами или энумами с осмысленными именами.
[uquote="ra9ust",url="/forum/viewtopic.php?p=3220033#p3220033"]Чем свитч лучше чем иф?[/uquote]
наглядностью в случаях выбора из множества возможных значений.
[uquote="ra9ust",url="/forum/viewtopic.php?p=3220033#p3220033"]Не понял про идентификаторы.[/uquote]
месиво из нормальный английских count, month, year, buzzer и транслита типа nochnik, lustra, opros сильно режет глаза.
[uquote="ra9ust",url="/forum/viewtopic.php?p=3220033#p3220033"]Но повторюсь, мне в нем всё понятно[/uquote]
тоже повторюсь - эта отмазка не принимается)

[uquote="ra9ust",url="/forum/viewtopic.php?p=3220033#p3220033"]**избавляйтесь от кучи одинаковых функций, все отличие которых - разные переменные. Сему способствует пункт 7.** и с этим согласен, но подскажите как? На простом примере.[/uquote]
вместо трешища

Код: Выделить всё

day_1_al, day_2_al,day_3_al,day_4_al,day_5_al,day_6_al,day_7_al,hour_al,min_al,hour_al_1,
hour_al_2,hour_al_3,hour_al_4,hour_al_5,hour_al_6,hour_al_7,min_al_1,min_al_2,
min_al_3,min_al_4,min_al_5,min_al_6,min_al_7

пишем чото типа

Код: Выделить всё

typedef struct {
	unsigned char day,
	unsigned char hour,
	unsigned char minute
} alarm_time_type;

alarm_time_type alarms[7];

и работаем дальше с массивом.

ra9ust

А вот вместо трешища короткая запись, это спасибо, примем к сведению. Ну а транслит и всё такое это просто на скорую руку, а потом так и осталось. А код правда мне понятен, ну видимо у кажного свое восприятие/написание.

smalcom

[uquote="ra9ust",url="/forum/viewtopic.php?p=3219963#p3219963"]Ну хорошо, наверняка здесь есть хорошо владеющие СИ, вот прокомментируйте сей код, насколько он кошмарный/некошмарный, само устройство на нем работает как я и задумывал, но может как-то можно код сократить/улучшить без потери функциональности? Если кому понадобится могу скинуть весь проект+схема в протеусе. Кто что скажет?[/uquote]
Мне сегодня сказали, что в четверг будет конец света - что-то там связанное с Юпитером и Венерой. Я не очень поверил поначалу, но теперь всё понял: не качайте архив!!!

shindax

Налицо ( как минимум

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

Код: Выделить всё

...
 if(sec>=60){min++;sec=0;}
 if(min>=60){hour++;min=0;tic=1;}
 if(hour==23&&min==59&&sec==50){if(cor>0)sec+=cor;} 
 if(hour==23&&min==59&&sec==59&&cc>11){if(cor<0)sec-=(128-(cor&0b01111111));cc=0;}
 if(hour>=24){day++;hour=0;week++;mint=0;maxt=0;eep_day=day;eep_month=month;eep_week=week;eep_year=year;}
 ...

Я-бы написал:

Код: Выделить всё

if(sec>=60)
 {
    min++;
    sec=0;
    if(min>=60)
    {
      hour++;
      min=0;
      tic=1;
 
       if(hour >= 24)
      {
        day ++;
        hour = 0;
        week ++;
        mint = 0;
        maxt = 0;
        eep_day = day;
        eep_month = month;
        eep_week = week;
        eep_year = year;
      }
    }
 
 }
 
 if( hour == 23 && min == 59 )
 {
  if( sec == 50 )
    if( cor > 0 )
      sec+=cor;
 
  if( sec == 59 && cc > 11 )
      {
        if(cor<0)
          sec-=(128-(cor&0b01111111));
        cc=0;
      }
  }

В целом, я-бы всё совсем по-другому написал.

Код, это просто п...ц, согласен с вышенаприведенными резюме.

Мне вот этого ( в одном месте функции, мля!!! )

Код: Выделить всё

...
 tal++;
 tr++;
 a++;
 b++;
 tpirr++;
 tpirrr++;
...

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

ra9ust

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

Если работает - не трогайте. Не нужно ничего переделывать.
Все рекомендации и советы будете принимать во внимание при следующем проекте.

PS: Опыт приходит со временем.

ra9ust

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

viiv

[uquote="ra9ust",url="/forum/viewtopic.php?p=3219963#p3219963"]вот прокомментируйте сей код, насколько он кошмарный/некошмарный[/uquote]

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

[uquote="ra9ust",url="/forum/viewtopic.php?p=3219963#p3219963"]само устройство на нем работает как я и задумывал[/uquote]
«Не верю!» (с) Станиславский

Вы просто плохо тестировали устройство

Спойлер

Код: Выделить всё

if((day==32&&month==1)||(day==29&&month==2)||(day==32&&month==3)||(day==31&&month==4)||
 (day==32&&month==5)||(day==31&&month==6)||(day==32&&month==7)||(day==32&&month==8)||
 (day==31&&month==9)||(day==32&&month==10)||(day==31&&month==11)){month++;day=1;}
 if(day==32&&month==12){month=1;day=1;year++;}

Вы специально сделали так, что в высокосные года Ваше устройство работало неправильно?

Спойлер

Код: Выделить всё

interrupt [TIM1_COMPA] void timer1_compa_isr(void)
{
 cc++;
 if(cc>=254)cc=14;
 sec++;
 if(sec>=60){min++;sec=0;}
 if(min>=60){hour++;min=0;tic=1;}
 if(hour==23&&min==59&&sec==50){if(cor>0)sec+=cor;} 
 if(hour==23&&min==59&&sec==59&&cc>11){if(cor<0)sec-=(128-(cor&0b01111111));cc=0;}
 if(hour>=24){day++;hour=0;week++;mint=0;maxt=0;
[b]/* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */[/b]
eep_day=day;eep_month=month;eep_week=week;eep_year=year;}
 if(week>=8)week=1;       
 
 if((day==32&&month==1)||(day==29&&month==2)||(day==32&&month==3)||(day==31&&month==4)||
 (day==32&&month==5)||(day==31&&month==6)||(day==32&&month==7)||(day==32&&month==8)||
 (day==31&&month==9)||(day==32&&month==10)||(day==31&&month==11)){month++;day=1;}
 if(day==32&&month==12){month=1;day=1;year++;} 

 if(PINC.5==0&&w==1)secur++; 
 if(pirkor==1)pirkorr++;
 if(start_back==1)back--;
 if(start_rele==1)rele++; 
 countdown=((time_ohr-1)-rele); 
 if(start_mic==1)count_mic++;
 if(start_pirkor==1)count_pirkor++; 
 if(start_gerkon_1==1)count_gerkon_1++;
 if(start_gerkon_2==1)count_gerkon_2++;
 if(start_pirtua==1)count_pirtua++;
 
 if(ind_mic==1)cou++;
 if(start_ohtik==1)ohtik++;
 if(start_nochnik==1)tnochnik++;
 
 if(start_tt==1)tt--; 
 if(tt<0){timer_count--;tt=59;} 
 
 if(start_tt_2==1)tt_2--; 
 if(tt_2<0){timer_count_2--;tt_2=59;} 
 
 tal++;
 tr++;
 a++;
 b++;
 tpirr++;
 tpirrr++; 
}

Странно, eep_month и eep_year присваивается ДО корректировки month и year (т.е. возможно неправильными значениями).

Пока_без_кота

Еще вопрос о правилах "хорошего тона". Можно писАть

Код: Выделить всё

 if (delay != 0) delay --;

а можно

Код: Выделить всё

 if (delay) delay --;

Насколько эстетичны такие сокращения с точки зрения "профессиональных любителей" ? Ну типа, фыркаете ли Вы, видя такое в чужом коде, или нормально относитесь ?

ra9ust

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

arkhnchul

зависит от. Общее правило - если нас интересует числовое значение (как в данном примере), то и пишем явное сравнение с числом.

ARV · Сообщение **ARV** » Пн окт 30, 2017 20:59:25

Пока_без_кота писал(а):Ну типа, фыркаете ли Вы, видя такое в чужом коде, или нормально относитесь ?

дело не в том, фыркают ли другие... дело в том, видите ли вы полезный смысл в написанном или нет. у каждого своё понимание перфекционизма, но любой перфекционизм ради перфекционизма плох.

если вы НИКОГДА не будете проверять В ЭТОМ МЕСТЕ delay на равенство чему-то отличному от нуля, смысла писать != 0 нет. но если логика программы или избранного вами алгоритма такова, что ВОЗМОЖНЫ варианты, когда delay может сравниваться с чем-то еще - то стоит задуматься... а не правильнее ли написать delay != DELAY_LIMIT, определив DELAY_LIMIT равным 0, но подразумевая, что там может быть и нечто иное?

а вот -- или ++ я лично никогда не отделяю от операнда пробелом

Добавлено after 5 minutes 21 second:
[uquote="ra9ust",url="/forum/viewtopic.php?p=3219963#p3219963"]Ну хорошо, наверняка здесь есть хорошо владеющие СИ, вот прокомментируйте сей код, насколько он кошмарный/некошмарный, само устройство на нем работает как я и задумывал, но может как-то можно код сократить/улучшить без потери функциональности? Если кому понадобится могу скинуть весь проект+схема в протеусе. Кто что скажет?[/uquote]