Is there a better way to write this for C++?

Question

The goal is to create substrings from an inputted string by extracting words which are separated by spaces.

The substrings have to be variables themselves.

Sounds easy, but what makes it hard is that you can only use strcpy, strcmp, strlen, strcat, strncpy, strncmp, strnlen, and strncat.

Example:

input:

"John 40 Hitman"

driver:

...

cout << word1 << endl 
     << word2 << endl 
     << word3 << endl;

output:

John
40
Hitman

---

Here is my code

#include <iostream>
#include <cstring>
#include <stdio.h>

int main(){

const char *string = "Name Age Job";
char name[10];
char age[10];
char job[10];
int length = strlen(string);
int temp = 0;
bool flag = false;

for(int i = 0; i < length + 1; i++){
  if(isspace(string[i]) && !flag){
    strncpy(name, string, i);
    name[i] = '\0';
    temp = i;
    flag = !flag;
    cout << name << endl;
    continue;
  }
  if(isspace(string[i])){
    strncpy(age, string + temp + 1, length - i - 1);
    age[temp - i] = '\0';
    temp = i;
    cout << age << endl;
    continue;
  }
  if(string[i] == '\0'){
    strncpy(job, string + temp + 1, length);
    job[temp - i] = '\0';
    cout << job << endl;
  }
}

It works but it has to use a flag boolean, strings are not dynamic, only works for a string with 2 spaces, and there is a lot of repeated code. Overall really janky, but I spent roughly two hours on this and I do not know how to improve it.

If you are wondering, this is indeed a homework problem, but it's an intro class and my professor only wants the correct output for a hard coded string with only 3 words. I, however, want to learn how to improve upon it and would be really grateful for any help. Thanks.

Answer 1

All you need to do is replace space ' ' with '\0'(the string end) thus creating 3 substrings from the original. The following program does that and just dumps the string to cout but you could hold the pointers in an array as well (eg. char* substring[3]).

int main(){
    char string[] = "Name Age Job";
    char* temp = string;
    for(char* it = string; *it; ++it ) {
        if (*it == ' ') {
            *it = '\0';
            std::cout << temp << std::endl;
            temp= it + 1;
        }
    }
    std::cout << temp << std::endl;
}

Answer 2

The proper way to do this with C functions only would be to use strtok, although that one chops up a string in-place.

Regarding your code, there's lot of needless branching and checks. You should avoid continue, it is an almost 100% certain sign of a loop in need of improvements. Whenever you find yourself needing it, there's usually a better way.

You should also avoid strncpy because as you've noticed, it is a pain to keep track of when it null terminates and when it doesn't. It's a dangerous function and much slower than memcpy, which could be used instead here.

Here is a simplified version based on using 2 pointers, one all the time set to point at the next space and one all the time set to point at the beginning of the next valid word.

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

const char* find_next_space (const char* str)
{
  while(*str != '\0' && !isspace(*str))
    str++;
  return str;
}

int main (void)
{
  const char str[] = "hello world how are you";
  char substr[5][10];


  const char* word_start = str;
  for(size_t i = 0; i<5; i++)
  {
    if(*word_start == '\0')
      break;

    const char* next_space = find_next_space(word_start); 

    size_t length = (size_t)(next_space-word_start);
    memcpy(substr[i], word_start, length); 
    substr[i][length] = '\0';
    puts(substr[i]);

    word_start = next_space+1;
  }
}

This code simplifies things by not checking if a string would fit and it doesn't allocate memory. Real production-quality C code wouldn't use char [5][10] but rather a pointer array char* [5] where each pointer is set to point at dynamically allocated memory.

Answer 3

I agree with @acraig5075 as your code is more C than C++. If you are thinking of writing this in C++ using the STL string, one way of doing this is the following.

#include <iostream>
#include <string>
#include <sstream>
#include <vector>


std::vector<std::string> split_string( const std::string &srcString, const char &delimiterKeywrd )
{
    /**
     *  Splits a string according to a keyword delimiter and returns a vector with the 
     *  segments of the split string.
     *  
     *  Args:
     *      @param  srcString:          The required string to split.
     *      @param  delimiterKeywrd:    The delimiter keyword that the string will be splitted to
     *      @return segmentsVectr:      A vector holding the segments of the srcString that was splitted. 
     *
    */

    std::stringstream inputStr;
    inputStr.str( srcString );
    std::string segment;

    std::vector<std::string> segmentsVectr;

    while( std::getline( inputStr, segment, delimiterKeywrd ) )
    {
        segmentsVectr.push_back( segment );
    }

    return segmentsVectr;
}


int main() {
    std::string inputStr{"John 40 Hitman"};

    std::vector<std::string> result;
    char delimiterKeywrd = ' '; 

    result = split_string( inputStr, delimiterKeywrd );

    //  Iterate through the vector and print items on a different line.
    for ( const std::string &item : result )
    {
        std::cout << item << std::endl;
    }

    return 0;
}

Check std::string at cppreference and examples if you are not familiar. Also here I am using std::stringstream which makes it easy to use std::getline.

If std::stringstream is not your preferred way you can always check this implementation from another question about splitting a string by character.

Hope this helps.