please review my code (splitting a char[])

[Thomas Peterson]

Hi,

I just wrote a simple demo-program that should split a char[] in several
pieces. It seems that it is working probably.
But now I like to know if it is good or if there are better ways for
doing what I want.


#include "stdafx.h"
#include "stdio.h"
#include "windows.h"

void split(char *string);

int main(void)
{
char test[30];
strcpy(test, "test.txt -r 123 -t 456 hello");
split(test);

Sleep(20000);

}

void split(char *string)
{
char *sub_string;
while ( (sub_string = strtok(string, " ")) != 0)
{
string = 0;

if(strcmp(sub_string, (char*)"-r") == 0)
{

sub_string = strtok(string, " ");
if(sub_string != 0)
{
printf("sub string -r = '%s'\n", sub_string);
string = 0;
}
}
else if(strcmp(sub_string, (char*)"-t") == 0)
{
string = 0;
sub_string = strtok(string, " ");
if(sub_string != 0)
{
printf("sub string -t = '%s'\n", sub_string);
string = 0;
}
}
}
}


Regards,
Thomas

 

[Phlip]

I just wrote a simple demo-program that should split a char[] in several
pieces. It seems that it is working probably.
But now I like to know if it is good or if there are better ways for
doing what I want.

Firstly, this code is C. If you indeed are learning C, switch to a C
newsgroup. This newsgroup will tempt you with C++, which provides better
libraries that make all these things much easier.

#include "stdafx.h"
#include "stdio.h"
#include "windows.h"

The last one is only needed if you create windows, which you don't, so now
it only slows down your compile. (You will take out the Sleep() below!)

void split(char *string);

I usually put the called function above main().

int main(void)
{
char test[30];
strcpy(test, "test.txt -r 123 -t 456 hello");

You can also just say char test[] = "my string", without the strcpy().

split(test);

Here, put assertions which check the string indeed got split.

Sleep(20000);

Use Google to learn how to keep your console up long enough to see its
output. There are better techniques than Sleep()

}

void split(char *string)
{ ....
if(sub_string != 0)
{
printf("sub string -r = '%s'\n", sub_string);

I will fade out here. This function couples output with processing. Ideally,
you would store the split strings in a container of some time, and return
them. Then the calling code would have the option to display them, or not.

 

[Selwyn Alcantara]

char test[30];

strcpy(test, "test.txt -r 123 -t 456 hello"); "

I would strongly suggest investing some time into learning the standard
string class, that would be the first obvious improvement. Not only is
it easier, it's safer.

#include <string>

int main()
{
std::string test = "test.txt -r 123 -t 456 hello"
....
return 0;
}

 

[Achintya]

Hi,

You are trying to do command line parsing. As said right by Phlip try
using existing C libraries for doing this.

-achintya