suggestion for writing a program to concatanate two strings

[sravishnu]

Hello,
I have written a program to concatanae two strings, and should be
returned to the main program. Iam enclosing the code,
please give me ur critics. Thanks,
main()
{
char s1[20],s2[20];
printf("enter first string");
scanf("%s",s1);
printf("enter second string");
scanf("%s",s2);
string_concatenate(&s1,&s2);
}
count_characters(char *x)
{
int i=0, count=0;
for(i=0;*(x+i)!='\0';i++)
count++;
return count;
}
string_concatenate(char *s1,char *s2)
{
int i,j;
char s3[50];
for(i=0;i<count_characters(s1);i++)
s3=s1;
for(j=0;j<count_characters(s2);j++)
s3[i+j]=s2[j];
s3[i+j]=s2[j];
printf("%s",s3);
}

 

[Jordan Abel]

Hello,

I have written a program to concatanae two strings, and should be
returned to the main program. Iam enclosing the code,
please give me ur critics. Thanks,

You are not Prince, please don't write like him - you pull it off
badly.

Also, your string_concatenate doesn't "return to the main program", it
prints its result. It in fact does not return anything at all.

 

[sravishnu]

i am sorry,
i know it is silly q.
but i want to print it in main program.
i want to return the array to main using address
can u change my code i am poor in pointers so i am practicing

 

[Keith Thompson]

i am sorry,
i know it is silly q.
but i want to print it in main program.
i want to return the array to main using address
can u change my code i am poor in pointers so i am practicing

Please read <http://cfaj.freeshell.org/google/> and follow its advice.

Please don't use silly abbreviations like "u" for "you" and "q" for
"question". It just makes your article more difficult to read. And
please use proper capitalization ("I", not "i", and the first letter
of a sentence).

I'm not saying this to be picky; it will make it easier for us to help
you.

 

[pete]

i am sorry,
i know it is silly q.
but i want to print it in main program.
i want to return the array to main using address
can u change my code i am poor in pointers so i am practicing

/* BEGIN new.c */

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

char *string_concatenate(char *s1, char *s2);

int main(void)
{
char s1[sizeof "concatenate string"] = "concatenate ";
char s2[] = "string";

puts(string_concatenate(s1, s2));
return 0;
}

char *string_concatenate(char *s1, char *s2)
{
strcpy(strlen(s1) + s1, s2);
return s1;
}

/* END new.c */

 

[sravishnu]

Thank you.
I want to write code with out using string handling functions.
So i wrote the code like that.
Now i just want to return it to tha main function.
once again thanks

 

[osmium]

I have written a program to concatanae two strings, and should be
returned to the main program. Iam enclosing the code,
please give me ur critics. Thanks,
main()
{
char s1[20],s2[20];
printf("enter first string");
scanf("%s",s1);
printf("enter second string");
scanf("%s",s2);
string_concatenate(&s1,&s2);
}
count_characters(char *x)
{
int i=0, count=0;
for(i=0;*(x+i)!='\0';i++)
count++;
return count;
}
string_concatenate(char *s1,char *s2)
{
int i,j;
char s3[50];
for(i=0;i<count_characters(s1);i++)
s3=s1;
for(j=0;j<count_characters(s2);j++)
s3[i+j]=s2[j];
s3[i+j]=s2[j];
printf("%s",s3);
}

I have tried to preserve what you wrote for the most part. I have indicated
most, but not all of the changes. It passes some trivial tests but I make
no guarantees. You can use a for or while loop and avoid the need to have
the count characters function. Try to improve what you have, this is quite
wasteful.

#include<stdio.h>

void string_concatenate(char *s1,char *s2, char* s3); /*change*/
int main() /*change*/
{
char s1[20],s2[20];
char s3[50];
/*change*/
printf("enter first string");
scanf("%s",s1);
printf("enter second string");
scanf("%s",s2);
string_concatenate(s1,s2, s3);
/* Note well! change above*/
printf(s3);
/*change*/
getchar(); /*ignore*/
getchar(); /*ignore*/
}
int count_characters(char *x)
{
int i=0, count=0;
for(i=0;*(x+i)!='\0';i++)
count++;
return count;
}
void string_concatenate(char *s1,char *s2, char* s3) /*change*/
{
int i,j;
/*char s3[50]; */
for(i=0;i<count_characters(s1);i++)
s3=s1;
for(j=0;j<count_characters(s2);j++)
s3[i+j]=s2[j];
/*s3[i+j]=s2[j]; change*/
s3[i+j] = '\0'; /*terminate the new string*/
/* change*/
}

 

[Chuck F.]

I have written a program to concatanae two strings, and should be
returned to the main program. Iam enclosing the code,

^^^ Sloppy, missing space

please give me ur critics. Thanks,

^^ Unknown word.

main()

illegal definition of main. Always of type int.

{
char s1[20],s2[20];
printf("enter first string");

Unknown action, no fflush executed.

scanf("%s",s1);

Calling another undefined function.
Buffer overrun vulnerability, no restriction on input size.
Failure to test success/failure of scanf call.

printf("enter second string");

See above

scanf("%s",s2);

and above, again

string_concatenate(&s1,&s2);

Calling undefined routine, very poor practice. Define any local
functions before you call them.

}

In general, lack of proper indentation, no separation between
routines, no description of routine purposes, lack of blanks in
actual statements, very poor style.

count_characters(char *x)
{
int i=0, count=0;
for(i=0;*(x+i)!='\0';i++)
count++;
return count;
}

I fail to detect any difference between the end action of this and
the end action of the _standard_ strlen() function.

string_concatenate(char *s1,char *s2)
{
int i,j;
char s3[50];

Why should 50 chars be enough space to concatenate two arbitrary
strings?

for(i=0;i<count_characters(s1);i++)
s3=s1;
for(j=0;j<count_characters(s2);j++)
s3[i+j]=s2[j];
s3[i+j]=s2[j];
printf("%s",s3);
}

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>

 

[Keith Thompson]

^^^ Sloppy, missing space

^^ Unknown word.


illegal definition of main. Always of type int.

It's legal in C90, but illegal in C99. (I mention this because the OP
might not believe it's illegal if his compiler doesn't complain about
it.)

"int main(void)" is better for both C90 and C99.

 

[Randy Howard]

(e-mail address removed) wrote
(in article

Thank you.
I want to write code with out using string handling functions.

I think you mean that your teacher wants you to do it that way.

 

[vire]

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

char* getCat(char* str1,char* str2)
{
int i=strlen(str1)+strlen(str2);
char* ret= new char(i);
ret[0]='\0';
strcat(ret,str1);
strcat(ret,str2);
return ret;
}

int main()
{
char a[20],b[20];
scanf("%s%s",a,b);
printf("%s\n",getCat(a,b));
return 0;
}

 

[Rouben Rostamian]

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

char* getCat(char* str1,char* str2)
{
int i=strlen(str1)+strlen(str2);
char* ret= new char(i);

Oh boy. You're in BIG trouble now. This is a C newsgroup, remember?

 

[Artie Gold]

vire wrote:

[failing to quote the relevant portions of the post to which he/she was
responding]

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

char* getCat(char* str1,char* str2)
{
int i=strlen(str1)+strlen(str2);

BZZZZT! And the terminating null character goes where?
ITYM:
int i = strlen(str1) + strlen(str2);

char* ret= new char(i);

....and, of course, you misspelled
char *ret = malloc(i);
if (ret) {

ret[0]='\0';
strcat(ret,str1);
strcat(ret,str2); }
return ret;
}

int main()
{
char a[20],b[20];
scanf("%s%s",a,b);

Bad karma -- a buffer overrun waiting to happen!

printf("%s\n",getCat(a,b));
return 0;
}

HTH,
--ag

 

[Keith Thompson]

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

char* getCat(char* str1,char* str2)
{
int i=strlen(str1)+strlen(str2);
char* ret= new char(i);

The "new" operator is C++, not C. Assuming it's intended to allocate
i bytes, you haven't allocated enough space for the trailing '\0'.

ret[0]='\0';
strcat(ret,str1);

These two statements could be replaced by

strcpy(ret, str1);

strcat(ret,str2);
return ret;
}

int main()
{
char a[20],b[20];
scanf("%s%s",a,b);

And what happens if the input is too long?

printf("%s\n",getCat(a,b));
return 0;
}

Also, please read <http://cfaj.freeshell.org/google/>.

 

[Richard Heathfield]

vire said:

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

char* getCat(char* str1,char* str2)
{
int i=strlen(str1)+strlen(str2);
char* ret= new char(i);
ret[0]='\0';
strcat(ret,str1);
strcat(ret,str2);
return ret;
}

int main()
{
char a[20],b[20];
scanf("%s%s",a,b);
printf("%s\n",getCat(a,b));
return 0;
}

foo.c: In function `getCat':
foo.c:7: `new' undeclared (first use in this function)
foo.c:7: (Each undeclared identifier is reported only once
foo.c:7: for each function it appears in.)
foo.c:7: parse error before `char'
foo.c:6: warning: unused variable `i'
make: *** [foo.o] Error 1

 

[Emmanuel Delahaye]

(e-mail address removed) a écrit :

I have written a program to concatanae two strings, and should be
returned to the main program. Iam enclosing the code,
please give me ur critics. Thanks,

First of all, I'd suggest

- A better trimming of your compiler:

Compiling: main.c
main.c:2: warning: return type defaults to `int'
main.c: In function `main_':
main.c:4: error: implicit declaration of function `printf'
main.c:4: warning: nested extern declaration of `printf'
<internal>:0: warning: redundant redeclaration of 'printf'
main.c:5: error: implicit declaration of function `scanf'
main.c:5: warning: nested extern declaration of `scanf'
<internal>:0: warning: redundant redeclaration of 'scanf'
main.c:8: error: implicit declaration of function `string_concatenate'
main.c:8: warning: nested extern declaration of `string_concatenate'
main.c: At top level:
main.c:11: warning: return type defaults to `int'
main.c:11: warning: no previous prototype for 'count_characters'
main.c:18: warning: return type defaults to `int'
main.c:18: warning: no previous prototype for 'string_concatenate'
main.c: In function `string_concatenate':
main.c:26: warning: nested extern declaration of `printf'
<internal>:0: warning: redundant redeclaration of 'printf'
main.c:27:2: warning: no newline at end of file
Process terminated with status 1 (0 minutes, 0 seconds)
3 errors, 10 warnings

- A better presentation (indentation) Use spaces instead of tabs.

You code commented. Please ask for details if you need it.

/* -ed- missing headers added */
#include <stdio.h>

/* -ed-
- code rearranged according to the

"define before use"

state-of-the-art rule

- static added to the non exported functions according to the

"scope shall be reduced to minimum"

state-of-the-art rule

*/

/* -ed-
count_characters(char *x)

- explicit return type added (mandatory in C99)
- this functions only reads the string. It may accept the address of
a read-only string. 'const' qualifier added.
*/
static int count_characters(char const *x)
{
int i = 0, count = 0;
/* -ed
for (i = 0;*(x + i) != '\0';i++)

*(x + i) is a complicated way of writing x
*/
for (i = 0; x != '\0'; i++)
{
/* -ed- { } added for maintenance facilities... */
count++;
}

/* -ed- do you really need count ? How is it different from i ? */
return count;
}
/* -ed-
Ok, I understand that you are learning C and that you are building your
own tools. But note that strlen() is part of the standard C library and
that is grossly achieves what you wanted here.
*/


static void string_concatenate(char *s1, char *s2)
{
int i, j;
char s3[50];
/* -ed-
how to be sure that 50 bytes is enough ?
Additionally, you never test the size in the function.
This is a serious potential bug.
*/

/* -ed-
for (i = 0;i < count_characters(s1);i++)

This is nuts. You recompute the length at each turn.
It is obvioulsy completely inefficient.

*/
int const len1 = count_characters(s1);

for (i = 0;i < len1;i++)
{
s3 = s1;
}

{
int const len2 = count_characters(s2);

for (j = 0;j < count_characters(s2);j++)
{
s3[i + j] = s2[j];
}
}

/* -ed- Ok you didn't forgot the final 0. Good point. */
s3[i + j] = s2[j];

/* -ed- debug : ,' added. Also, \n added for reasons exposed further */
printf("'%s'\n", s3);
}

int main()
{
char s1[20], s2[20];

/* -ed-
printf("enter first string");

Warning. To be sure that the text comes out, you must terminante the
string with a \n or force the output with fflush(stdout).
Also, for a better presentation, I suggest to add ": "

*/
printf("enter first string: ");
fflush(stdout);

/* -ed-
scanf("%s", s1);

scanf() is a hard-to-use-it-right function.
Even advanced programmers avoid it like plague...

The way you use it is dangerous. It behaves like gets(), hence there
is no input limit control.

For a quick-and-dirty code you can use it as follow, but it's
ugly and
hard to maintain... Also, the returned value should be tested,
and there
are issues with string containing blanks and eventually with pending
characters, I'm not sure... (I don't use scanf())

A better solution to get a line is to use fgets(), it's not trivial,
but at least, it can be used safely.

*/
scanf("%19s", s1);

printf("enter second string: ");
fflush(stdout);
scanf("%19s", s2);

/* -ed-
string_concatenate(&s1, &s2);

Wrong type. According to the prototype, you want to pass the
address of
a char (here, the address of the 1st element of the array),
hence &s1[0], that is also written s1 + 0 or more simply s1.

Ditto for s2.

*/
string_concatenate(s1, s2);

/* explicit return in main() added (mandatory in C90) */
return 0;
}

 

[Richard Heathfield]

Chuck F. said:

^^ Unknown word.

See "Jingo", by Terry Pratchett.

 

[Richard Heathfield]

(e-mail address removed) said:

Hello,
I have written a program to concatanae two strings, and should be
returned to the main program. Iam enclosing the code,
please give me ur critics. Thanks,

I suggest you pay lots of attention to the other critiques by respected
regular contributors to this newsgroup. In addition, I advise you never to
use an unadorned %s as a part of a scanf format string. It provides no
protection against arbitrarily large amounts of data being loaded into
memory - a classic "buffer overrun attack" vulnerability.

Algorithmically, however, you have approximately the right idea.

 

[Michael R. Copeland]

i am sorry,

Please don't use silly abbreviations like "u" for "you" and "q" for
"question". It just makes your article more difficult to read. And
please use proper capitalization ("I", not "i", and the first letter
of a sentence).

Amen, brother!

 

[Simon Biber]

vire wrote:

[failing to quote the relevant portions of the post to which he/she was
responding]

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

char* getCat(char* str1,char* str2)
{
int i=strlen(str1)+strlen(str2);

BZZZZT! And the terminating null character goes where?
ITYM:
int i = strlen(str1) + strlen(str2);

I fail to see any meaningful difference. I think you meant to add a one
in there. Also, the variable should be of type size_t.

...and, of course, you misspelled
char *ret = malloc(i);

That will require that you #include <stdlib.h>, which I didn't see you
mention.