Prośba o pomoc przy przeróbce kodu na taki z wykorzystaniem wskaźników

Elvis · Marzec 6, 2022

Chodziło mi tylko o to. Albo raczej nie do końca "tylko".

Zdecydowałeś się na użycie tablicy do zapisywania wartości minimalnej i maksymalnej, czyli indeks 0 to minimum, a 1 to maksimum. To oczywiście działa, ale jest raczej niezalecaną praktyką. Teraz pamiętasz co z znaczy 0, a co 1, ale jeśli będziesz chciał zmieniać swój program za jakiś czas, może być ciężko sobie przypomnieć o co chodziło. Drugi problem to łatwość pomyłki - co właśnie przetestowałeś. Użycie 0 zamiast 1 łatwo przeoczyć, gdyby nie eleganckie formatowanie kodu, pewnie jeszcze długo nikt tego kodu by nie zauważył.

Więc pierwsza poprawka, którą warto przemyśleć to rezygnacja z "magicznych liczb" - czyli liczb, które mają znaczenie znane tylko piszącemu program.

Używasz wyliczać (enum), więc nieco poprawiony program mógłby wyglądać następująco:

enum{grzejniki, cwu, piec, slonce, grunt, zew, wilg, cisnienie, LICZBA_CZUJNIKOW=8};

enum{temp_min, temp_max, TEMP_N=2};

int odczyty[8];

struct sensors{
  int temp, dzien, miesiac, rok;
};

struct sensors czujniki[LICZBA_CZUJNIKÓW][TEMP_N];

void saveTemp2(){

  for(i=0; i<LICZBA_CZUJNIKÓW; i++{

    if (odczyty[i] < czujniki[i][temp_min].temp) saveTemp(&czujnik[i][temp_min], &odczyty[i]);
    if (odczyty[i] > czujniki[i][temp_max].temp) saveTemp(&czujnik[i][temp_max], &odczyty[i]);
}}

Przy okazji warto przemyśleć nazywanie stałych. Zwyczajowo pisze się je dużymi literami, z "enum" bywa różnie, są zwolennicy pisania tylko pierwszej litery dużej, w każdym razie kod, który jest teraz nie ułatwia zrozumienia, że np. "grzejniki" to stała, a nie zmienna.

Proponowałbym zmianę, na:

enum{GRZEJNIKI, CWU, PIEC, SLONCE, GRUNT, ZEW, WILG, CISNIENIE, LICZBA_CZUJNIKOW=8};

enum{TEMP_MIN, TEMP_MAX, TEMP_N=2};

Niezależnie od konwencji nazewniczej, używanie tablic będzie średnio piękne. Może lepiej zastąpić je strukturą?

Edytowano Marzec 6, 2022 przez Elvis

Elvis · Marzec 6, 2022

Edit: chyba trochę źle zrozumiałem program, jeśli chcesz zapisać datę odczytu minimalnego i maksymalnego, powinno być nieco inaczej.

Taki program pojawił się wcześniej, zaproponowany przez kolegę @ethanak-a, ale został zakrzyczany "dobrymi" poradami.

Chodzi o oddzielenie wartości od samego czujnika. Mamy więc strukturę opisującą wynik:

struct pomiar {
  int dzien, miesiac, rok;
  int temp;
};

Czujnik przechowuje dwa wyniki pomiaru: minimalny i maksymalny, czyli:

struct sensors{
  struct pomiar min;
  struct pomiar max;
};

Teraz tablica czujników jest 1-wymiarowa, po prostu każdy czujnik przechowuje 2 wyniki:

struct sensors czujniki[LICZBA_CZUJNIKÓW];

Obliczanie minimum i maksimum mogłoby mieć taką postać:

void saveTemp2(){

  for(i=0; i<LICZBA_CZUJNIKÓW; i++{
    
    if (odczyty[i] < czujniki[i].min.temp) saveTemp(&czujnik[i].min, &odczyty[i]);
    if (odczyty[i] > czujniki[i].max.temp) saveTemp(&czujnik[i].max, &odczyty[i]);
}}

Funkcja saveTemp powinna przyjmować wskaźnik do pomiaru, ale to chyba nie problem.

Pomysł z globalną tablicą odczyty[] jest raczej niezbyt udany, więc może warto wrócić do porad kilka stron temu... o ile pamiętam, było tam w wiele lepsze rozwiązanie.

Ale poprawiać kod można w nieskończoność - to co było moim zdaniem błędne, to użycie indeksów 0 i 1, co łatwo pomylić, albo zapomnieć.

farmaceuta · Marzec 6, 2022

49 minut temu, Elvis napisał:

Taki program pojawił się wcześniej, zaproponowany przez kolegę @ethanak-a, ale został zakrzyczany "dobrymi" poradami.


struct pomiar {
  int dzien, miesiac, rok;
  int temp;
};


struct sensors{
  struct pomiar min;
  struct pomiar max;
};


struct sensors czujniki[LICZBA_CZUJNIKÓW];


void saveTemp2(){

  for(i=0; i<LICZBA_CZUJNIKÓW; i++{
    
    if (odczyty[i] < czujniki[i].min.temp) saveTemp(&czujnik[i].min, &odczyty[i]);
    if (odczyty[i] > czujniki[i].max.temp) saveTemp(&czujnik[i].max, &odczyty[i]);
}}

Zapewne o moje dobre porady Ci chodzilo...wiec dalej sie upieram ze kod idzie w zlym kierunku i wiele jest tu nie potrzebnych linijek😉 ale co kto zrobi to juz dla mnie bez roznicy...

Elvis · Marzec 6, 2022

Warto jeszcze pomyśleć o początkowych wartościach zmiennych. Tablica czujniki[] jest globalna, więc jej pola będą inicjalizowaine wartościami "0". To niby dobra wartość, ale w przypadku np. temperatury minimalnej może być problemem - program nie zapisze nowego minimum dopóki nie przyjdą mrozy...

Program można napisać na mnóstwo sposobów, ja proponowałbym dodanie 2 funkcji. Pierwszej, która będzie inicjalizowała odczyt z czujnika:

void init_sensor(struct sensor* sens, int value)
{
    saveTemp(&sens->min, value);
    saveTemp(&sens->max, value);
}

Jest to ogólnie kod z wcześniejszej pętli, ale bez if-ów. Czyli najpierw wykonujemy odczyt z każdego czujnika, a później wywołujemy init_sensor - pierwsza wartość minimalna i maksymalna to nasz pierwszy pomiar.

Można też napisać funkcję, która będzie aktualizowała wyniki, czyli:

void update_sensor(struct sensor* sens, int value)
{
    if (value < sens->min.temp)
        saveTemp(&sens->min, value);
    if (value > sens->max.temp)
        saveTemp(&sens->max, value);
}

Wywołanie init_sensor oraz update_sensor może być w pętli, ale wcale nie musi. Nie wiem, jak w tym przypadku, ale wiele programów dokonuje pomiarów z różną częstotliwością - więc nie trzeba aktualizować wszystkich czujników za każdym razem, wystarczy te, które mają nowe dane.

Produkcja i montaż PCB - wybierz sprawdzone PCBWay!
   • Darmowe płytki dla studentów i projektów non-profit
   • Tylko 5$ za 10 prototypów PCB w 24 godziny
   • Usługa projektowania PCB na zlecenie
   • Montaż PCB od 30$ + bezpłatna dostawa i szablony
   • Darmowe narzędzie do podglądu plików Gerber
Zobacz również » Film z fabryki PCBWay

ethanak · Marzec 6, 2022

3 minuty temu, Elvis napisał:

nie trzeba aktualizować wszystkich czujników za każdym razem, wystarczy te, które mają nowe dane.

Dlatego pierwsza moja wersja saveTemp2() była przeznaczona do zapisania pojedynczego wyniku, dopiero później nie wiadomo czemu stworzone zostały jakieś globalne tablice odczytów i jakieś pętle...

Elvis · Marzec 6, 2022

Z tymi pętlami będzie jeszcze jeden problem - jeśli wszystkie czujniki są identyczne i można je odczytywać wywołaniem tej samej funkcji, to użycie pętli ma sens. Ale jeśli czujniki są różne, każdy wymaga innego kodu i może być odczytywany w różnych momentach, to pętla jest raczej bez sensu.

Więc chyba lepiej tworzyć funkcje działające na danych dla jednego czujnika przydadzą się zarówno dla pętli, jak i oddzielnych wywołań.

SOYER · Marzec 6, 2022

2 minuty temu, Elvis napisał:

Z tymi pętlami będzie jeszcze jeden problem - jeśli wszystkie czujniki są identyczne i można je odczytywać wywołaniem tej samej funkcji, to użycie pętli ma sens. Ale jeśli czujniki są różne, każdy wymaga innego kodu i może być odczytywany w różnych momentach, to pętla jest raczej bez sensu.

Więc chyba lepiej tworzyć funkcje działające na danych dla jednego czujnika przydadzą się zarówno dla pętli, jak i oddzielnych wywołań.

Jak wcześniej pisałem, odczyty z czujników mam(na urządzeniu które realizuje ten program) pod postacią zmiennych(ew. tablicy zmiennych) zaciągniętych z innych urządzeń realizujących pomiar.

SOYER · Marzec 6, 2022

enum{GRZEJNIKI, CWU, PIEC, SLONCE, GRUNT, ZEW, WILG, CISNIENIE, LICZBA_CZUJNIKOW=8};
enum{TEMP_MIN, TEMP_MAX, TEMP_N=2};

int odczyty[8];

struct pomiar{
  int temp, dzien, miesiac, rok;
};

struct sensors{
  struct pomiar minimum;
  struct pomiar maximum;
}
struct sensors czujniki[LICZBA_CZUJNIKÓW];

void saveTemp(struct sensors *wsk,  int *odczyt){
  
  wsk->temp = odczyt;
  wsk->dzien = now.day();
  wsk->miesiac = now.month();
  wsk->rok = now.year();
}
void saveTempInit(){
  for(i=0; i<LICZBA_CZUJNIKÓW; i++{
    saveTemp(&czujnik[i].minimum, odczyty[i]);
    saveTemp(&czujnik[i].maximum, odczyty[i]);
  }
}

void saveTemp2(){

  for(i=0; i<LICZBA_CZUJNIKÓW; i++{

    if (odczyty[i] < czujniki[i].minimum.temp) saveTemp(&czujnik[i].minimum, odczyty[i]);
    if (odczyty[i] > czujniki[i].maximum.temp) saveTemp(&czujnik[i].maximum, odczyty[i]);
}}

??

SOYER · Marzec 7, 2022

Dobrze czy nie?

Bo chciałbym ew. następny temat założyć😀.

farmaceuta · Marzec 8, 2022

Powinno wszystko dzialac z pominieciem kilku brakow,srednik, nawias etc...i jak masz wskaznik w funkcji to musisz podac adres a nie wartosc...teraz masz tak

odczyty[i]

A powinno byc

&odczyty[i]

Elvis · Marzec 8, 2022

void saveTemp(struct sensors *wsk,  int *odczyt){

Pierwszym parametrem powinien być wskaźnik na strukturę pomiar - sensors ma teraz dwa pomiary, program nie wiedziałby który ma zapisać.

Drugi parametr lepiej zrobić jako zwykły int, bez wskaźnika. Wskaźnik nic tutaj nie daje, a wygodniej jest używać funkcji, która przujmuje parametr jako wartość, można wtedy np. wywołać:

saveTemp(&termometr.minimum, 25);

Chyba nie kompilowałeś jeszcze kodu, tutaj raczej nie zgadzają się nawiasy:

for(i=0; i<LICZBA_CZUJNIKÓW; i++{

Niepotrzebne jest przypisanie wartości do stałej LICZBA_CZUJNIKOW:

enum{GRZEJNIKI, CWU, PIEC, SLONCE, GRUNT, ZEW, WILG, CISNIENIE, LICZBA_CZUJNIKOW=8};

Dodawanie takiej stałej to bardzo nieeleganckie rozwiązanie, ale skoro już jest to niech chociaż kompilator ustala jej wartość - teraz jeśli dodasz albo usuniesz czujnik, a zapomnisz ustawić wartość LICZBA_CZUJNIKOW pojawi się dość nieprzyjemny błąd.

Inna sprawa, że w kodzie pojawia się podobna, ale inna stała o nazwie LICZBA_CZUJNIKÓW.

Pewnie jeszcze długo można byłoby się czepiać... Proponuję spróbować skompilować ten program, pewnie wtedy będzie widać ile trzeba poprawić.

Edytowano Marzec 8, 2022 przez Elvis

Prośba o pomoc przy przeróbce kodu na taki z wykorzystaniem wskaźników

Pomocna odpowiedź

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Polecacz 101

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Link do komentarza

Share on other sites

Dołącz do dyskusji, napisz odpowiedź!

Najnowsze posty w innych tematach