Il vostro parere su due stili di programmazione

[TardoFreak]

Buona sera a tutti i forumisti di ElectroYou,
Vorrei sottoporre alla vostra attenzione due modi per scrivere una funzione. Si tratta di una funzione che serve per far lampeggiare (o no) due LED a seconda dello stato di funzionamento della macchina.
Occorre tenere presente questi importanti aspetti:
- I LED sono utilizzati solo in questa funzione. In nessun altra parte del programma i LED vengono accesi o spenti.
- L' hardware e' rigido. Si sa gia' in partenza che i LED sono accesi con uno stato logico 1 e spenti con uno stato logico 0. Non si potra' mai pensare il contrario. Cosi' sono punto e basta. E anche se si collegassero ad altri pin di uscita sarebbe sempre e comunque accesi con uno stato logico 1 e spenti con uno 0. L' hardware lo impone.

STILE #1

Codice: Seleziona tutto

   #define mInitAllLEDs()      LATD &= 0xFFF8; TRISD &= 0xFFF8;
   
    #define mLED_1              LATDbits.LATD0
    #define mLED_2              LATDbits.LATD1             
   
    #define mGetLED_1()         mLED_1
    #define mGetLED_2()         mLED_2

    #define mLED_1_On()         mLED_1 = 1;
    #define mLED_2_On()         mLED_2 = 1;       
   
    #define mLED_1_Off()        mLED_1 = 0;
    #define mLED_2_Off()        mLED_2 = 0;     
   
    #define mLED_1_Toggle()     mLED_1 = !mLED_1;
    #define mLED_2_Toggle()     mLED_2 = !mLED_2;

void BlinkUSBStatus(void)
{
    static WORD led_count=0;
   
    if(led_count == 0)led_count = 10000U;
    led_count--;

    #define mLED_Both_Off()         {mLED_1_Off();mLED_2_Off();}
    #define mLED_Both_On()          {mLED_1_On();mLED_2_On();}
    #define mLED_Only_1_On()        {mLED_1_On();mLED_2_Off();}
    #define mLED_Only_2_On()        {mLED_1_Off();mLED_2_On();}

    if(USBSuspendControl == 1)
    {
        if(led_count==0)
        {
            mLED_1_Toggle();
            if(mGetLED_1())
            {
                mLED_2_On();
            }
            else
            {
                mLED_2_Off();
            }
        }//end if
    }
    else
    {
        if(USBDeviceState == DETACHED_STATE)
        {
            mLED_Both_Off();
        }
        else if(USBDeviceState == ATTACHED_STATE)
        {
            mLED_Both_On();
        }
        else if(USBDeviceState == POWERED_STATE)
        {
            mLED_Only_1_On();
        }
        else if(USBDeviceState == DEFAULT_STATE)
        {
            mLED_Only_2_On();
        }
        else if(USBDeviceState == ADDRESS_STATE)
        {
            if(led_count == 0)
            {
                mLED_1_Toggle();
                mLED_2_Off();
            }//end if
        }
        else if(USBDeviceState == CONFIGURED_STATE)
        {
            if(led_count==0)
            {
                mLED_1_Toggle();
                if(mGetLED_1())
                {
                    mLED_2_Off();
                }
                else
                {
                    mLED_2_On();
                }
            }//end if
        }//end if(...)
    }//end if(UCONbits.SUSPND...)

}//end BlinkUSBStatus

STILE #2

Codice: Seleziona tutto

#define mInitAllLEDs()      LATD &= 0xFFF8; TRISD &= 0xFFF8;
   
#define mLED_1              LATDbits.LATD0
#define mLED_2              LATDbits.LATD1

void BlinkUSBStatus(void)
{
    static unsigned short led_count=0;
   
    if(led_count == 0)led_count = 10000U;
    led_count--;

    if(USBSuspendControl == 1)
    {
      if(led_count==0)
      {
        mLED_1 = !mLED_1;
        mLED_2 = mLED_1;
      }
    }
    else
    {
      if(USBDeviceState == DETACHED_STATE)      { mLED_1 = 0; mLED_2 = 0; }
      else if(USBDeviceState == ATTACHED_STATE) { mLED_1 = 1; mLED_2 = 1; }
      else if(USBDeviceState == POWERED_STATE)  { mLED_1 = 1; mLED_2 = 0; }
      else if(USBDeviceState == DEFAULT_STATE)  { mLED_1 = 0; mLED_2 = 1; }
      else if(USBDeviceState == ADDRESS_STATE)
      {
        if(led_count == 0)
        {
            mLED_1 = !mLED_1;
            mLED_2 = 0;
        }
      }
      else if(USBDeviceState == CONFIGURED_STATE)
      {
        if(led_count==0)
        {
          mLED_1 = !mLED_1;
          mLED_2 = !mLED_1;
        }
      }
    }
}

La domanda e': quale dei due stili considerate migliore e per quale motivo?

E' solo semplice curiosita'.

[DonJ]

Per una funzione del genere io userei la seconda.

Non cambia nulla alla compilazione, quindi credo che qui si tratti puramente di "gusto" nella stesura del listato.
Sceglierei la seconda perché è più leggibile, le operazioni da fare sono poche e banali e la lettura è sicuramente migliore.

Tutto qui.
Spero di aver interpretato bene la tua domanda

[DirtyDeeds]

Anch'io sceglierei la seconda più o meno per gli stessi motivi di DonJ, anche se la prima è più genrale: però io sono eoni che non programmo, quindi non faccio testo

Piuttosto, come mai non hai usato uno switch al posto di tutti quegli if... else?

[simo85]

È vero, e se non erro lo switch è più veloce.

[TardoFreak]

Anche io avrei usato lo switch.
La discussione verte sullo stile.

[c1b8]

Io preferisco la prima soluzione.
Il motivo: essere il più possibile generici ed avere la possibilità di modificare velocemente il software; per questo le tue premesse "importanti" non valgono poi tanto nella scelta.
E' vero che "questo" hardware impone certe scelte, ed infatti nel software si usano delle define per definire costanti, ma è anche vero che cambiando, un domani, hardware o semplicemente le specifiche software, mi farebbe piacere riutilizzare il software senza o con un numero minimo di modifiche.
Già cambiare hardware mi imporrà la riprogettazione dello stesso, perché dover riprogettare/spendere tempo anche nel software?
In alcuni punti del codice, ad esempio, accendi/spegni il led2 in funzione del led1, oggi questo è molto semplice; un domani potrebbe dipendere da condizioni più complesse come dallo stato del led1 e/o dallo stato di un pin: nel primo caso la modifica è semplice, nel secondo molto meno.

[angus]

La prima soluzione. Straquoto c1b8 sul restare "generici" per avere la possibilità di riutilizzare il codice.

Da parziale ignorante della programmazione dei micro e consapevole che forse il codice sarebbe stato meno ottimizzato, sarei stato ancora più generico scrivendo una funzione che accende/spegne/inverte il led lasciando la decisione su QUALE led agire esterna alla funzione, passandole uno o più parametri. È da matti?

[g.schgor]

o sarei più pragmatico: quale dei due richiede
meno tempo di esecuzione?
La cosa non è banale perché ho tentato di programmare
lo SCU in Java la funzione Fad_In e Fad-Out ed ho constatato
tempi di esecuzione impossibili !

[TardoFreak]

Sono identiche al 99%, cambia solo lo stile di scrittura.

[c1b8]

Generalizzare troppo ha i suoi svantaggi
Ciò che bisogna però tenere a mente, secondo me, è che le specifiche hardware non devono influenzare la scelta dello stile di programmazione e che si debba cercare di generalizzare il più possibile.
Il caso portato da g.schgor ne è un esempio: tempi di esecuzione del software nulla hanno a che vedere con le specifiche hardware di led fissi o non fissi ecc.