Validating a tab-delimited string

Question

This code checks if string has correct format for passing to struct. Correct format needs to be [123456789'\t'First Name'\t'Last Name'\t'City]. I didn't know how to enter tabs, so I used '\t'. I use them to separate different word blocks so e.g. first and middle name can be read to a first name in struct.

I wrote alternative version as well (in comments). Which one is more preferable? How is the correct form to write nested ternary operator

int checkString(char *string, int len)
{


    char *ch;
    int flag = FALSE;
    int i = 0;

    ch = string;

    i = 0;
    /*
    while (ch[i] && ch[i] != '\t') {
        if (isdigit(ch[i])) {
            if (++i == len) {
                flag = ch[i] == '\t' ? TRUE : FALSE;
            }
        } else { ch++; }
    }
    */

    /* ID code needs to be 11 digits */
    for (i = 0; ch[i] && ch[i] != '\t';
        isdigit(ch[i]) ? 
        flag = ++i == len ?             // while true
        ch[i] == '\t' ? TRUE : FALSE    // while true
        : FALSE 
        : *ch++);

    /* Three tabs is correct */
    if (flag) {
        for (i = 0; ch[i];
        ch[i] == '\t' ?
        flag = ++i == 3 ? TRUE : FALSE
        : *ch++);
    }

    return flag;
}

Answer 1

Avoid nested ternary operators. It hardly ever improves readability over proper if-else conditions.

When the boolean values true and false are involved, often there's simply no need for tensors at all, like in this example:

flag = ch[i] == '\t' ? TRUE : FALSE;

This is equivalent and simpler:

flag = ch[i] == '\t';

When a condition gets complicated, another technique is to extract the complex boolean expression to a helper method with a good descriptive name.

Answer 2

The commented version is certain better in terms of readability, even if it doesn't work, because ch++ return the value of the memory address of the string first char plus one.

For the id field:

while (ch[i] && ch[i] != '\t' && i < id_length) {
    if (!isdigit(ch[i++])) {
        flag--;
        break;
    }
}

//pass id_length to the function
if (i == id_length && ch[i] == '\t') flag++;   

I suggest you to set id_length as an optional argument with a default value ID_LENGTH, where ID_LENGTH is a const you have to define, for example as = 11.

For the others field:

while (ch[i] && ch[i] != '\t' && i < x_length) {   // where x is your field
    if (!(isalpha(ch[i]) || ch[i++] == ' ')) {
        flag--;
        break;
    }
}

if (i == x_length && ch[i] == '\t') flag++;

I suggest you to use the tab also at the end of the 'city' field to make easier for you to check more fields with the same functions and in general to be able to extend the action of your functions to the new possible elements of your struct.

Also remember to include <ctype.h> for isdigit() and isalpha() functions.

Since with your actual code, you can't collect informations about what specific field were wrong formatted, you can directly decrement the flag also for the final '\t' checks and check if flag is equal to zero at the end.

Otherwise, you cas use flag as a real FLAG and increment the flag for each succes by 2 ** (field position), so that your function will return a detailed infos about each field.

unsigned int position = 0;

while (ch[i] && ch[i] != '\t' && i < id_length) {
    if (!isdigit(ch[i++])) {
        break;
    }
}

if (i == id_length) flag += 1 << position++;


/* For each other field */
while (ch[i] && ch[i] != '\t' && i < x_length) {
    if (!(isalpha(ch[i]) || ch[i++] == ' ')) {
        break;
    }
}

if (i == x_length) flag += 1 << position++

And a final possible implementation of the function would be:

/* 
 *   field_lengths:
 *       array that contains the expected length of each struct field to check
 */
int checkString(char *ch, unsigned int len, unsigned int *field_lengths)   
{
    unsigned int flag = NULL;
    unsigned int position = 0;
    unsigned int expected_point = 0;
    unsigned int i = 0;

    while (ch[i] && ch[i] != '\t' && i < id_length) {
        if (!isdigit(ch[i++])) {
            break;
        }
    }

    /* retrieve the current field length */
    id_length = field_lengths[position]; 
    /* prepare the initial start point for the next check */
    expected_point += id_length + 1;

    /*
     *   x << n shifts x of n bits, so it's equal to x * pow(2, n) 
     *   1 << n shif 1 of n bits, so flag += 1 << n set n-th bit of flag
     */
    if (i == id_length) flag += 1 << position++;
    /* 
     *   increment position so that the new info 
     *   will be stored in an higher bit of flag
     */


    /* For each other field: */
    i = expected_point;
    while (ch[i] && ch[i] != '\t' && i < expected_point + x_length) {
        if (!(isalpha(ch[i]) || ch[i++] == ' ')) {
            break;
        }
    }
    x_length = field_lengths[position];
    expected_point += x_length + 1;
    if (i == x_length) flag += 1 << position++;

    return flag;
}

Answer 3

Call me crazy if you will (after all, I am), but I'd do most of this with sscanf:

char const format[] = "%11[0-9]%1*[\t]%30[^\t]%1*[\t]%30[^\t]%1*[\t]%30[^\t]%1*[\t]";

if (4 == sscanf(string, format, ID, first_name, last_name, city) && strlen(ID) == 11)
    // valid data

So let's tear apart that format string into the individual pieces, so maybe they're not quite so...opaque.

It's really a number of separate pieces:

%11[0-9]

This says read (up to) 11 digits. It's not as well known as (I think) it deserves, but scanf supports a small but useful subset of regular expressions in what are called scan-set conversions. Technically, this one has undefined behavior--the standard doesn't require that 0-9 actually means the same as 0123456789. If you look really hard, you can find a compiler (Borland on MS-DOS) that will actually interpret this as matching the three characters 0, - and 9. If you really have a reason to care about ancient compilers, you'll want to use 0123456789 explicitly instead.

%1*[\t]

This says to read only one character, which can only be a tab. The * means "don't assign the result to a variable", so we're just verifying that what's there is a tab character.

Note that this is some extra complexity because of a minor oddity in how scanf (and company) work in general. If you were using a non-whitespace character as your separator (e.g., :) you could check for a match by just putting that character in the format string. For example, scanf("(%d:%d)", &a, &b); will only match two numbers inside of parentheses and separated by a colon.

Whitespace is special though: any whitespace (space, tab, vertical tab, new-line, carriage return) in the format string will match an arbitrary amount of whitespace in the input (so, for example, one space can match two tabs followed by a new-line, three more tabs, and a carriage return).

So, the scan-set is just to prevent that special white-space behavior from kicking in here, which could allow (for example) four spaces as a separator instead of the tab that you want.

%30[^\t]

This says to read up to 30 characters (feel free to increase if necessary), that cant't contain a tab. Unlike the - in the previous scan set, the ^ at the beginning of the scan set is defined to mean "not any of", just like in a normal regular expression.

From there, we basically just repeat the last two conversions a few more times to read the rest of the fields.

The return from scanf (fscanf, vfscanf, etc.) is the number of items that were successfully converted (and assigned). Since we're looking for four fields, we compare to four.

Once we're done with that, we have one piece of verification left: this would/will still match if the ID (the first field) is fewer than 11 characters, so we also need to verify that strlen(id) == 11 to be sure it was all correct.

Other than that, a few minor details. First, since you're not planning to modify the string, you probably want to pass a char const *.

Second, C has had an actual Boolean type since C99, so you unless you need compatibility with ancient compilers, you might as well return a bool instead of an int. There room for a little variation here though. The reserved word that's directly recognized by the compiler is _Bool. Then there's a <stdbool.h> that defines bool (which expands to _Bool), true and false.

So, I'd write the code something like this:

#include <stdbool.h>

bool checkString(char const *string, int len) { 
    char ID[12];
    char first_name[31];
    char last_name[31];
    char city[31];

    char const format[] = "%11[0-9]%1*[\t]%30[^\t]%1*[\t]%30[^\t]%1*[\t]%30[^\t]%1*[\t]";

    return 4 == sscanf(string, format, ID, first_name, last_name, city) 
       && 11 == strlen(ID));
}

One minor addition: if you were really worried about/bothered by temporarily storing the variables holding the pieces of the string, you could actually skip all but the first and the last. You need to store the first (ID) because you want to check its length when you're done reading, and you want to save the last to be sure that everything before it converted correctly. Here you're depending on the fact that scanf and company will simply quit if something doesn't convert correctly, so the last can succeed if and only if all the preceding succeeded as well.

Answer 4

Check a string's format using ternary operator

The below does not address ternary operator usage as it is not the best tool for the job. Other approaches include using strtok() and sscanf().

@Jerry Coffin good answer suggests using sscanf() to do the syntax check. Below is a similar method. It uses "%n" to record the offset of the scan.

Formats can get ugly. Recommend breaking them up.

int checkString(const char *string) {
#define NUMF "%*[0-9]"
#define TABF "%*1[\t]"
#define STRF "%*[^\t]"

  char const format[] = NUMF "%n" TABF STRF TABF STRF TABF STRF "%n";

  int num_len;
  int n = 0;
  sscanf(string, format, &num_len, &n);

  /// All fields scanned? number is 11 digit? No extra garbage
  return (n > 0 && num_len == 11 && string[n] == '\0');
}