0

I'm writing an NMEAParser library. As its name suggests, it parses NMEA sentences. Nothing crazy.

Its entry point is a function that accepts an NMEA string as its only parameter and looks at its beginning to pass it to the right decoder. Here is the function:

bool NMEAParser::dispatch(const char *str) {
    if (!str[0]) {
        return false;
    }

    //check NMEA string type
    if (str[0] == '$') {
        //PLSR245X
        if (str[1] == 'P' && str[2] == 'L' && str[3] == 'S' && str[4] == 'R' && str[5] == ',' && str[6] == '2' && str[7] == '4' && str[8] == '5' && str[9] == ',') {
            if (str[10] == '1')
                return parsePLSR2451(str);
            if (str[10] == '2')
                return parsePLSR2452(str);
            if (str[10] == '7')
                return parsePLSR2457(str);
        } else if (str[1] == 'G' && str[2] == 'P') {
            //GPGGA
            if      (str[3] == 'G' && str[4] == 'G' && str[5] == 'A')
                return parseGPGGA(str);
            //GPGSA
            else if (str[3] == 'G' && str[4] == 'S' && str[5] == 'A')
                return parseGPGSA(str);
            //GPGSV
            else if (str[3] == 'G' && str[4] == 'S' && str[5] == 'V')
                return parseGPGSV(str);
            //GPRMC
            else if (str[3] == 'R' && str[4] == 'M' && str[5] == 'C')
                return parseGPRMC(str);
            //GPVTG
            else if (str[3] == 'V' && str[4] == 'T' && str[5] == 'G')
                return parseGPVTG(str);
            //GPTXT
            else if (str[3] == 'T' && str[4] == 'X' && str[5] == 'T')
                return parseGPTXT(str);
            //GPGLL
            else if (str[3] == 'G' && str[4] == 'L' && str[5] == 'L')
                return parseGPGLL(str);
        }
        //HCHDG
        else if (str[1] == 'H' && str[2] == 'C' && str[3] == 'H' && str[4] == 'D' && str[5] == 'G')
            return parseHCHDG(str);
    }
    return false;
}

The problem I have is that this function's cyclomatic complexity is quite high, and my SonarQube complains about it: cyclomatic complexity

It's not really a problem as the code is quite easy to read. But I was wondering how I could reduce its complexity while still keeping it simple to read and efficient.

X99
  • 905
  • 10
  • 24
  • Using `memcmp` or `strncmp` instead of a bunch of character comparison may help. – rodrigo Jun 13 '20 at 14:35
  • I think it probably is all the character comparisons that drive up this metric, as suggested by @rodrigo. As an alternative to `memcmp` or `strncmp`, you could use the `string` class. – Dennis Sparrow Jun 13 '20 at 14:44

1 Answers1

0

You can simplify this quite a lot:

if (std::string_view{str, 10} == "$PLSR,245,")
{
  switch (str[10])
  {
    case '1' : return parsePLSR2451(str);
    case '2' : return parsePLSR2452(str);
    case '7' : return parsePLSR2457(str);
  }
}
else if (std::string_view{str + 1, 2} == "GP")
{
  auto s = std::string_view{str + 3, 3};
  if (s == "GGA")
    return parseGPGGA(str);
  if (s == "GSA")
    return parseGPGSA(str);
  // ... etc
}
else if (std::string_view{str + 1, 5} == "HCHDG")
{
   return parseHCHDG(str);
}

return false;

There's no extra strings being constructed either, so it should be at least as efficient.

cigien
  • 57,834
  • 11
  • 73
  • 112