C program to convert a sum of money to Indian Number Format

Question

I wanted to build a C program to convert any number to Indian Currency Format. It can take inputs with negative sign, leading zeros and Decimal point + Mantissa and it will format the input to take care of the leading zeros and add the commas according to Indian number format. For example, numbers in Indian Number Format are represented as:

Minus sign: "-12345" -> "-12,345"
Decimal Point: "-12345.123" -> "-12,345.123"
Leading Zero's: 000000.123 → "0.123" or "-000123456.1234" -> "-1,23,456.1234"

I would like to optimise this code in terms of time and space complexity, make it more concise and clean if possible.

The basic logic of adding the commas is to first copy the sign character if it exists to the second array and then remove it from the original number array. Then if the number of digits in the Exponent part is odd then 1 digit is copied to the second array and removed to make the length even. then for every odd value of the index 'i' a comma is added.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *convertToInrFormat(char *lp_number, int lv_l)
{
    char *lp_fnumber = '\0';
    char *lp_p = '\0';
    int lv_index = 0;
    int lv_nsize = 0;
    int lv_zerocount = 0;
    int sign_count = 0;
    int expo_count = 0;
    int lv_i = 0;
    int lv_j = 0;

    if (lp_number[0] == '-') // The 0th position of the char array is checked for negative sign, if found sign_count is incremented.
    {
        sign_count++;
    }

    lv_zerocount = strspn(lp_number + sign_count, "0"); // the no. of Leading Zeros is calculated ignoring the negative sign if present.

    if (lp_number[sign_count + lv_zerocount] == '.') //if the exponent part consists of only 0's then the zerocount is reduced by 1 to leave behind 1 zero in the exponent part.
    {
        lv_zerocount = lv_zerocount - 1;
    }

    if (lv_zerocount > 0) //the zeros are removed by being overwritten
    {
        memmove(lp_number + sign_count, lp_number + lv_zerocount + sign_count, strlen(lp_number));
    }

    while (lp_number[sign_count] != '.' && lp_number[sign_count] != '\0') //the count of remaining exponents is taken
    {
        expo_count++;
        sign_count++;
    }

    lv_l = strlen(lp_number); // New string length

    if (expo_count > 3) //inserting the commas
    {
        lv_nsize = lv_l + (expo_count / 2 - 1) + 1;
        lp_fnumber = (char *)malloc(lv_nsize);

        if (lp_fnumber != NULL)
        {
            if (lp_number[0] == '-')
            {
                lp_fnumber[0] = lp_number[0];
                lv_j++;
                memmove(lp_number, lp_number + 1, strlen(lp_number));
                lv_l--;

                if (expo_count % 2 != 0)
                {
                    lp_fnumber[1] = lp_number[0];
                    lv_j++;
                    memmove(lp_number, lp_number + 1, strlen(lp_number));
                    expo_count--;
                }
            }
            else if (expo_count % 2 != 0)
            {
                lp_fnumber[0] = lp_number[0];
                lv_j = lv_j + 1;
                memmove(lp_number, lp_number + 1, strlen(lp_number));
                expo_count--;
            }

            lp_p = strchr(lp_number, '.');
            if (lp_p != NULL)
            {
                lv_index = lp_p - lp_number;
            }

            while (lv_i < expo_count)
            {
                lp_fnumber[lv_j++] = lp_number[lv_i++];

                if (lv_i + 2 < expo_count && lv_i % 2 != 0) //Alt logic:((lv_i ^ lv_l) & 1) here for every odd value of i index a comma is added.
                    lp_fnumber[lv_j++] = ',';
            }
            if (lv_index != 0)
            {
                while (lp_number[lv_index] != '\0')
                {
                    lp_fnumber[lv_j++] = lp_number[lv_index++];
                }
            }
            lp_fnumber[lv_j] = '\0';
        }
        return lp_fnumber;
    }
    else
    {
        return lp_number;
    }
}

   int main()
{
    char lp_number[255];
    int lv_l;
    char *formated_number;
    printf("Enter the lp_number\n");
    fgets(lp_number, 255, stdin);
    lv_l = strlen(lp_number);
    formated_number = convertToInrFormat(lp_number, lv_l);
    puts(formated_number);
}

Answer 1

1. In C it's more idiomatic to initialize pointers to NULL instead of '\0'
2. Don't cast the return value of malloc

3. Here

   lv_nsize = lv_l + (expo_count / 2 - 1) + 1;
   lp_fnumber = (char *)malloc(lv_nsize);
   

   After that you use lv_j as index into the return string but you never check if lv_j stays within the allocated bounds. If there is a bug somewhere in the code or the input is formatted in a way you didn't expect then this could lead to a buffer overflow which causes undefined behaviour and is one of the main reasons hackers get access to systems.

4. Not sure what the purpose of prefixing almost all variable with lp or lv is - smells bit of the widely misunderstood hungarian notation and doesn't make the code any clearer.

5. You can reduce nesting a bit by checking the return value of malloc and return immediately, like this:

   lp_fnumber = malloc(lv_nsize);
   if (!lp_fnumber)
   {
       return NULL;
   }
   

6. This if-else block:

       if (lp_number[0] == '-')
       {
           lp_fnumber[0] = lp_number[0];
           lv_j++;
           memmove(lp_number, lp_number + 1, strlen(lp_number));
           lv_l--;
   
           if (expo_count % 2 != 0)
           {
               lp_fnumber[1] = lp_number[0];
               lv_j++;
               memmove(lp_number, lp_number + 1, strlen(lp_number));
               expo_count--;
           }
       }
       else if (expo_count % 2 != 0)
       {
           lp_fnumber[0] = lp_number[0];
           lv_j = lv_j + 1;
           memmove(lp_number, lp_number + 1, strlen(lp_number));
           expo_count--;
       }
   

   can be simplified to:

       int has_sign = lp_number[0] == '-';
       if (has_sign)
       {
           lp_fnumber[0] = lp_number[0];
           lv_j++;
           memmove(lp_number, lp_number + 1, strlen(lp_number));
           lv_l--;
       }
   
       if (expo_count % 2 != 0)
       {
           lp_fnumber[has_sign ? 1 : 0] = lp_number[0];
           lv_j++;
           memmove(lp_number, lp_number + 1, strlen(lp_number));
           expo_count--;
       }
   

7. You perform a lot of memmove operation - instead you could simply advance a pointer to the character you want to continue on. That would also get rid of a lot of the indexing.

8. The variable naming isn't always the best lp_number and lp_fnumber don't really tell me much plus they are also very similar which makes understanding the code a bit tedious. source and dest or input and result would make it a lot clearer what is what.

Answer 2

Not bad.

Consider using the += operator, as well as using printf instead of puts.

I would suggest making using printf for any output a habit.