HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcMinor

Least significant digit (LSD) radix sort of strings

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lsdradixleastsortdigitstringssignificant

Problem

I am learning string algorithms and wrote this C example based on a Java example I found online. Any feedback relating to style, code clarity, whether it is generic enough to be re-used, interface, etc, etc would be very much appreciated.

/*
 Demonstration of least significant digit (LSD) radix sort of strings
 Specifically UK vehicle registration numbers
*/
#include 
#include 
#include 

#define PLATELENGTH 7
#define ALPHABET 36

/* Characters must be uppercase or digits */
void lsd_string_sort(char* a[], const int length, int string_size) {
   int R = ALPHABET;
   int W = string_size;
   const int N = length;
   char id = 0;
   char** aux = (char**)malloc(length * string_size);

   for(int d = W-1; d >= 0; d--) {
      int* count = (int*)calloc(R+1, sizeof(int));
      /* whether character or digit */
      id = (*(a[0]+d) >= '0' && *(a[0]+d)  0)
      free(plates[--n]);

   return 0;
}

Solution

-
This macro name is misleading:

#define ALPHABET 36


The alphabet consists of 26 letters, not 36. But in a CS context, this can be implied (in this case, it appears to include A-Z and 0-9).

However, this macro is storing the alphabet's size, not the alphabet itself. The name should clearly specify that. A more appropriate name could be ALPHABET_LENGTH.

On another note, the other macro should be renamed to PLATE_LENGTH. Underscores are commonly used to separate each word in a macro.

-
Avoid single-character variable names:

int R = ALPHABET;
int W = string_size;


Based on the context (fortunately), I can tell that these hold sizes. Other than that, they are very unclear and could lead to maintenance problems at some point. Always give variables descriptive names, unless they're simple loop counters.

-
You have the list-displaying code in main() twice. Why not make it a function? This will reduce the amount of code in main() and maintain DRY, making it clearer to read and to maintain.

void display(char list[], int n)
{
    for (int i = 0; i < n; i++)
        printf("%s\n", plates[i]);
}


You could keep the label outputs in main(), right before the function calls.

Code Snippets

#define ALPHABET 36
int R = ALPHABET;
int W = string_size;
void display(char list[], int n)
{
    for (int i = 0; i < n; i++)
        printf("%s\n", plates[i]);
}

Context

StackExchange Code Review Q#47009, answer score: 9

Revisions (0)

No revisions yet.