C Implementazione di atof

Question

Sono un principiante in C. Attualmente sto implementando atof per costruire un raytracer, tuttavia sto ancora imparando a scrivere programmi in modo efficiente.

Assegnazione

Istruzioni

Il programma prende un file di descrizione della scena come argomento per generare oggetti. Alcuni di questi parametri sono float. Esempio del file.

Sto analizzando il file. Dato che sono limitato in termini di linee consentite per funzione e attualmente sto imparando come funzionano i doppi puntatori, sto usando un puntatore a doppio carattere. Esempio di una di queste funzioni che utilizza lc_atof.

int    a_parsing(char *str, t_pars *data)
{
    if (*(str++) == 'A')
    {
        if (((data->a_ratio = lc_atof(&str)) >= 0.0) && data->a_ratio <= 1.0 && errno == 0)
        // 
            if (((data->a_R = lc_atoi(&str)) >= 0) && data->a_R <= 255 && errno == 0)
                if (*(str++) = ',' && ((data->a_G = lc_atoi(&str)) >= 0) && data->a_G <= 255 && errno == 0)
                    if (*(str++) = ',' && ((data->a_B = lc_atoi(&str)) >= 0) && data->a_B <= 255 && errno == 0)
                        return (skip_space(&str));
    }
    return (0);
}

Codice attuale da rivedere:

#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <limits.h>
#include <errno.h>
#include <float.h>

static float    conversion(char **str)
{
    double    d_nbr;
    double    power;

    d_nbr = 0.0;
    power = 10.0;
    while (isdigit(**str))
    {
        d_nbr = d_nbr * 10.0 + (**str - 48);
        if (d_nbr > FLT_MAX)
        {
            errno = EIO;
            return (-1);
        }
        (*str)++;
    }
    if (**str == '.')
    {
      (*str)++;
      if (isdigit(**str))
      {
        d_nbr = d_nbr * 10.0 + (**str - 48);
        if (d_nbr > FLT_MAX)
        {
            errno = EIO;
            return (-1);
        }
        (*str)++;
        return ((float)(d_nbr / power));
      }
    }
    errno = EIO;
    return (-1);
}

float            lc_atof(char **str)
{
    float    n;
    int        sign;

    n = 0.0;
    sign = 1.0;
    if (!str || !*str)
    {
        errno = EIO;
        return (-1);
    }
    while (isspace(**str))
        (*str)++;
    if (**str == '+' || **str == '-')
    {
        if (**str == '-')
            sign = -1.0;    
        (*str)++;
    }
    if (!isdigit(**str))
    {
        errno = EIO;
        return (-1);
    }
    if ((n = conversion(str)) == 0 && errno != 0)
       return (-1);
    return (sign * n);
}

Le uniche modifiche all'attuale atof che ho apportato sono avere un puntatore a doppio carattere come argomento e restituire -1 in caso di errori.

Ogni input molto apprezzato.

Answer 1

Portabilità

Non vi è alcuna garanzia che questo codice utilizzerà ASCII, quindi sarebbe meglio utilizzare '0'piuttosto che 48che è qualcosa di un numero magico. L'utilizzo '0'lo rende più leggibile e più facile da capire.

lc_atofNon gestisce correttamente la terminazione della stringa o la fine della riga

Questo codice non gestisce una stringa con terminazione NULL o un carattere di fine riga. La funzione isspace()ritorna trueper la fine della riga in modo che il codice la oltrepassi.

    while (isspace(**str))
        (*str)++;
    if (**str == '+' || **str == '-')
    {
        if (**str == '-')
            sign = -1.0;
        (*str)++;
    }
    if (!isdigit(**str))
    {
        errno = EIO;
        return (-1);
    }

Complessità

Mi rendo conto che non hai chiesto di rivedere questo, ma la complessità di ogni ifaffermazione nella funzione di chiamata di esempio è eccessiva e mi ha causato un errore nella mia recensione in precedenza:

int a_parsing(char* str, t_pars* data)
{
    if (*(str++) == 'A')
    {
        if (((data->a_ratio = lc_atof(&str)) >= 0.0) && data->a_ratio <= 1.0 && errno == 0)
            // 
            if (((data->a_R = lc_atoi(&str)) >= 0) && data->a_R <= 255 && errno == 0)
                if (*(str++) = ',' && ((data->a_G = lc_atoi(&str)) >= 0) && data->a_G <= 255 && errno == 0)
                    if (*(str++) = ',' && ((data->a_B = lc_atoi(&str)) >= 0) && data->a_B <= 255 && errno == 0)
                        return (skip_space(&str));
    }
    return (0);
}

Riscriverei il codice come:

#define MAX_COLOR   0xFF
int a_parsing_prime(char* str, t_pars* data)
{
    if (*(str++) == 'A')
    {
        data->a_ratio = lc_atof(&str);
        if (!errno && data->a_R <= MAX_COLOR)
        {
            if (*(str++) = ',')
            {
                data->a_G = lc_atoi(&str);
                if (!errno && data->a_G <= MAX_COLOR)
                {
                    if (*(str++) = ',')
                    {
                        data->a_B = lc_atoi(&str);
                        if (!errno && data->a_B <= MAX_COLOR)
                        {
                            return (skip_space(&str));
                        }
                    }
                }
            }
        }
    }
    return (0);
}

che mostra veramente la complessità della funzione.

Answer 2

• La scelta EIOper la segnalazione degli errori è molto dubbia. lc_atofnon esegue alcun input o output; perché dovrebbe segnalare un errore IO? Se il tipo restituito non può rappresentare il risultato (ad esempio d_nbr > FLT_MAX), una scelta logica è ERANGEo EOVERFLOW. Se la conversione non può essere completata a causa dell'argomento errato (ad esempio !isdigit(**str)), la scelta logica potrebbe essere EINVAL.

  Detto questo, non approvo l'impostazione errnonella funzione libreria. Una tradizione di lunga data è quella di impostare errnosolo le chiamate di sistema. So che questa tradizione è sempre più violata in questi giorni, ma ancora. Se disponi di altri mezzi di segnalazione degli errori, mantienili.

• L'uso del parametro inout ( strnel tuo caso) non è consigliabile. Complica inutilmente il codice, sia dal lato del chiamante che da quello del chiamato. Il chiamato è costretto a usare troppi riferimenti indiretti extra ea preoccuparsi delle parentesi (**str)++. A sua volta, il chiamante perde traccia di dove è iniziato il parsable (ad esempio, deve registrare il numero errato). Guarda come strtofgestisce questo:

    float strtof(const char *restrict nptr, char **restrict endptr);
  

  Ecco nptrun in-only ed endptrè out-only.

• Sono sorpreso che tu abbia deciso di limitare l'utilità della funzione gestendo solo una cifra dopo il punto decimale. Non è un grande sforzo gestirli tutti e i vantaggi sono molto maggiori.

• Non è necessario mettere tra parentesi il valore restituito. returnè un operatore, non una funzione.