a little mistake

A

ash

i am writing this program (for exercise1-9 in k&r-2nd edition) which
removes extra spaces in string
example- "test string" will be "test string"

#include<stdio.h>
#include<string.h>
#include<conio.h>
main()
{
char str[20],st[20];
int i=0,j=0;
printf("enter string");
gets(str);

while(str!=NULL)
{
if(str==' ')
{ st[j++]=st[i++];

while(str==' ') i++;

}
else
st[j++]=str[i++];
}
st[j]=NULL;
strcpy(str,st);
puts(str);
}

(compiled in turbo c++ for windows version 3.1)
i know there should be other ways to write this program (thats a bad
program ),can someone help me in writing that program in compact way
and give me some logic and tips.
i want to use gcc but i don`t have linux operating system and one
software which allows gcc compiling in windows (cygwin) is not working.
anyone can advice me such a program that allows compiling with gcc in
windows( this is off topic but advices are always welcome).

thankx
 
M

Malcolm

ash said:
i am writing this program (for exercise1-9 in k&r-2nd edition) which
removes extra spaces in string
example- "test string" will be "test string"

#include<stdio.h>
#include<string.h>
#include<conio.h>
main()
{
char str[20],st[20];
int i=0,j=0;
printf("enter string");
gets(str);

while(str!=NULL)
{
if(str==' ')
{ st[j++]=st[i++];

while(str==' ') i++;

}
else
st[j++]=str[i++];
}
st[j]=NULL;
strcpy(str,st);
puts(str);
}

(compiled in turbo c++ for windows version 3.1)
i know there should be other ways to write this program (thats a bad
program ),can someone help me in writing that program in compact way
and give me some logic and tips.
i want to use gcc but i don`t have linux operating system and one
software which allows gcc compiling in windows (cygwin) is not working.
anyone can advice me such a program that allows compiling with gcc in
windows( this is off topic but advices are always welcome).

thankx

Separate out ewhat you are doing into a function.

In this case you want

/*
turns whitespace spans into single spaces
(and trims leading / trailing white space?)
*/
void onespace(char *out, char *in)

Decide if you want to allow out and in to be the same. This will make your
function
more convenient to use, but you might have to be a little bit more careful
in coding it.

Basically you maintain two pointer, a "write" pointer and a "read" pointer.
If you hit
a non-whitespace you write to the "write" pointer and increment both.
If you hit a whitespace character, you write a single space to the "write"
pointer. Then you increment the "read" pointer whilst it is still pointing
to white space.
 
S

santosh

ash wrote:
.... snip ...
i want to use gcc but i don`t have linux operating system and one
software which allows gcc compiling in windows (cygwin) is not working.
anyone can advice me such a program that allows compiling with gcc in
windows( this is off topic but advices are always welcome).

Generally Cygwin is a bit complex to set up and use. Use MinGW instead.
It uses native Win32 ports of the GNU toolchain.

http://www.mingw.org/
 
K

Keith Thompson

ash said:
i am writing this program (for exercise1-9 in k&r-2nd edition) which
removes extra spaces in string
example- "test string" will be "test string"

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

<conio.h> is not a standard header. You don't use it anyway. Delete
the above line.

I find it clearer to leave a space between "include" and "<".

int main(void)
{
char str[20],st[20];
int i=0,j=0;
printf("enter string");

Since this doesn't end in a newline, it's not guaranteed to appear.
Add "fflush(stdout);".

I'd add a blank, or perhaps ": ", at the end of the prompt; it looks
better. Without it, if I type "foo", I see "enter stringfoo" on the
screen.
gets(str);

Never use gets(). Never use gets(). Never use gets().

See question 12.23 in the comp.lang.c FAQ said:
while(str!=NULL)


NULL is a null pointer constant. You're trying to compare a character
value to a pointer value. It happens to work on your system for
obscure reasons I won't get into, but it's bad style at the very
least. The way to represent a null character is '\0'.
{
if(str==' ')
{ st[j++]=st[i++];


I think you mean "st[j++]=st[i++];"
while(str==' ') i++;

}
else
st[j++]=str[i++];
}
st[j]=NULL;


Again, use '\0', not NULL.
strcpy(str,st);
puts(str);

return 0;

And don't underestimate the importance of consistent formatting,
particularly indentation.

Apart from that, the program doesn't seem unreasonable.
 
D

Default User

ash said:
i am writing this program (for exercise1-9 in k&r-2nd edition) which
removes extra spaces in string
example- "test string" will be "test string"

That's not the actual exercise:

Exercise 1-9. Write a program to copy its input to its output,
replacing each string of one or more blanks by a single blank.

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

main()

Use the following form:

int main(void)
{
char str[20],st[20];

These fixed buffers are likely a problem.
int i=0,j=0;
printf("enter string");
gets(str);

Never, ever use gets(). It's dangerous. What happens when someone
enters a string with 50 characters? Bad things.
while(str!=NULL)


NULL is a null pointer constant. It might be 0, which would work, but
it could be (void*)0. Either use '\0' or 0 in checking for the null
terminator in a string.
{
if(str==' ')
{ st[j++]=st[i++];

while(str==' ') i++;

}
else
st[j++]=str[i++];
}
st[j]=NULL;
strcpy(str,st);
puts(str);
}

(compiled in turbo c++ for windows version 3.1)
i know there should be other ways to write this program (thats a bad
program ),can someone help me in writing that program in compact way
and give me some logic and tips.
i want to use gcc but i don`t have linux operating system and one
software which allows gcc compiling in windows (cygwin) is not
working. anyone can advice me such a program that allows compiling
with gcc in windows( this is off topic but advices are always
welcome).


Do you want the program that K&R assigned, or the one you tried? It can
be made much more simple by just immediately outputting characters
instead of copying to buffers.

/* algorithm portion */

int c;
int flag = 0;

while ((c = getchar()) != EOF)
{
if (c == ' ')
{
if (flag == 0)
{
putchar(c);
flag = 1;
}
}
else
{
if (flag == 1)
{
flag = 0;
}
putchar(c);
}
}





Brian
 
C

CBFalconer

ash said:
.... snip ...

i want to use gcc but i don`t have linux operating system and one
software which allows gcc compiling in windows (cygwin) is not
working. anyone can advice me such a program that allows compiling
with gcc in windows( this is off topic but advices are always
welcome).

djgpp, at <http://www.delorie.com>. However cygwin should work.
 
J

Julian V. Noble

Malcolm said:
ash said:
i am writing this program (for exercise1-9 in k&r-2nd edition) which
removes extra spaces in string
example- "test string" will be "test string"

#include<stdio.h>
#include<string.h>
#include<conio.h>
main()
{
char str[20],st[20];
int i=0,j=0;
printf("enter string");
gets(str);

while(str!=NULL)
{
if(str==' ')
{ st[j++]=st[i++];

while(str==' ') i++;

}
else
st[j++]=str[i++];
}
st[j]=NULL;
strcpy(str,st);
puts(str);
}

(compiled in turbo c++ for windows version 3.1)
i know there should be other ways to write this program (thats a bad
program ),can someone help me in writing that program in compact way
and give me some logic and tips.
i want to use gcc but i don`t have linux operating system and one
software which allows gcc compiling in windows (cygwin) is not working.
anyone can advice me such a program that allows compiling with gcc in
windows( this is off topic but advices are always welcome).

thankx

Separate out ewhat you are doing into a function.

In this case you want

/*
turns whitespace spans into single spaces
(and trims leading / trailing white space?)
*/
void onespace(char *out, char *in)

Decide if you want to allow out and in to be the same. This will make your
function
more convenient to use, but you might have to be a little bit more careful
in coding it.

Basically you maintain two pointer, a "write" pointer and a "read" pointer.
If you hit
a non-whitespace you write to the "write" pointer and increment both.
If you hit a whitespace character, you write a single space to the "write"
pointer. Then you increment the "read" pointer whilst it is still pointing
to white space.


I think you also need a little state machine: if you have not hit
a blank you are in character-accepting mode and write that char
to the buffer where you construct the compacted string. Otherwise
if the char is a blank you accept it but shift to a mode where you
no longer accept blank characters. When you hit a non-blank you
switch back to the first mode. In other words,

state\input -> non-blank blank
// --------------------------------------------------------
1 write to buff write to buff and
set state to 2

2 write to buff and drop char
set state to 1
// --------------------------------------------------------

A switch statement and a state variable will accomplish this.
 
L

lovecreatesbeauty

ash said:
can someone help me in writing that program in compact way
and give me some logic and tips.


I don't know whether my program is compact or not, anyway code below
it's my own. Suggestion welcomed.


/* Removes extra spaces in string pointed by src, the result string is
pointed by dst. Return value is the length os the result string or
0 for failure. example: "test string" will be "test string" */
int spacerm(char *dst, char *src)
{
int last_space = 0; /*1: last char is a space char; 0: not*/
int len = 0;
int ret = 1; /*for return value, 0: failure, non-zero: result
string length returned*/

if (src == 0 || dst == 0)
{
ret = 0;
}

if (ret != 0)
{
while (*src != '\0')
{
if (*src == ' ')
{
if (last_space != 1)
{
*dst++ = *src++;
len++;
}
else
{
src++;
}

last_space = 1;

}
else
{
*dst++ = *src++;
len++;
last_space = 0;
}
}

ret = len;
}

return ret;
}

#include <stdio.h>
#include <string.h>
int main(void)
{
char *s = "test string";
char a[20] = {'\0'};
int i = 0;

printf("%i, %s\n", strlen(s), s);
i = spacerm(a, s);
printf("%i, %s\n", i, a);

return 0;
}

$ gcc -W -Wall -pedantic -ansi test.c
$ ./a.out
14, test string
11, test string
$

i want to use gcc but i don`t have linux operating system and one
software which allows gcc compiling in windows (cygwin) is not working.
anyone can advice me such a program that allows compiling with gcc in
windows( this is off topic but advices are always welcome).


Use mingw or Install a Debian Linux on VMware hosted in Windows.

lovecreatesbeauty
 
P

Peter Nilsson

lovecreatesbeauty said:
/* Removes extra spaces in string pointed by src, the result string is
pointed by dst. Return value is the length os the result string or
0 for failure. example: "test string" will be "test string" */

What if the original string is "". Is that an error? If so, why?
int spacerm(char *dst, char *src)
{
int last_space = 0; /*1: last char is a space char; 0: not*/
int len = 0;
int ret = 1; /*for return value, 0: failure, non-zero: result
string length returned*/

You effectively have two variables for the same thing. This
looks clumsy.
if (src == 0 || dst == 0)

Why bother? [Rehtorical]
{
ret = 0;
}

if (ret != 0)
{
while (*src != '\0')
{
if (*src == ' ')
{
if (last_space != 1)

if (last_space == 0)

Apart from being more efficient on many systems, it's
obvious what the test is. When you test explicitly against
a value like 1, the reader may pause to ponder whether
the variable can other values like 2 or 3 and why 1 is
significant.
{
*dst++ = *src++;
len++;
}
else
{
src++;
}

last_space = 1;

}
else
{
*dst++ = *src++;
len++;
last_space = 0;
}
}

You never write a terminating null byte to dst.
ret = len;
}

return ret;
}

#include <stdio.h>
#include <string.h>
int main(void)
{
char *s = "test string";
char a[20] = {'\0'};

Try...

char a[20] = "xxxxxxxxxxxWhat?";
int i = 0;

printf("%i, %s\n", strlen(s), s);

strlen() returns a size_t value; %i expects an int.

In C99 use %zu to print a size_t. In C90, cast the value
to suitable unsigned type and use %u with an appropriate
length modifier.
i = spacerm(a, s);
printf("%i, %s\n", i, a);

return 0;
}

$ gcc -W -Wall -pedantic -ansi test.c

Not a blip eg? Safe as houses then.

As some non-expert whose comments aren't worth the
em waves they're transmitted in once said "You control
 
F

Frederick Gotham

ash posted:
i am writing this program (for exercise1-9 in k&r-2nd edition) which
removes extra spaces in string
example- "test string" will be "test string"


I'd probably approach it something like:

#include <cstddef>

void RetreatStrXPlaces(register char *p, std::size_t const places)
{
/* Consider coding this by copying int's
and checking if any byte is a space
character */

if ( !places ) return;

register char *q = p - places;

while ( *q++ = *p++ );
}

void FixMultipleSpaces( char * const p_start )
{
for(char *p = p_start; ; )
{
switch( *p++ )
{
case 0: return;

case ' ':
{
unsigned places = 0;

while( *p++ == ' ' ) ++places;

RetreatStrXPlaces(--p, places);
}
}
}
}

#include <iostream>
#include <cstdlib>

int main()
{
using std::cout;

char buffer[] = "The man walked over the revine";

FixMultipleSpaces(buffer);

cout << buffer << '\n';

std::system("PAUSE");
}
 
R

Richard Heathfield

Frederick Gotham said:
ash posted:



I'd probably approach it something like:

#include <cstddef>

No such header.
void RetreatStrXPlaces(register char *p, std::size_t const places)

Unknown identifier, std
Syntax error: "::"

Just the first three of many.
 
J

Joe Estock

ash said:
i am writing this program (for exercise1-9 in k&r-2nd edition) which
removes extra spaces in string
example- "test string" will be "test string"

Sounds simple enough. The portable solution would be to encase this
functionality into a function so that you can easily utilize it among
other utilities, but for now I'll keep it simple.
#include<stdio.h>
#include<string.h>
#include<conio.h>

DOS nonsense. This is a non-standard header and looking at your code
below you use nothing in it. Get rid of it.

main returns a value. _ALWAYS_.
{
char str[20],st[20];
int i=0,j=0;
printf("enter string");

The user might not ever see this. If you really want to suppress the
newline (which ensures that the data gets written to the screen) then
consider calling fflush(stdout) immediately after.
gets(str);

Don't use gets(). This function is dangerous. There is a replacement
that one of the regulars of this newsgroup posted a while ago that is a
much safer alternative. Personaly I would use fgets but anything that
accepts a width parameter would be marginally better.
while(str!=NULL)


NULL is a pointer (in most cases it's 0 cast to a pointer to void, but
that's beside the point). While this may work on your system and perhaps
many other systems this is a recipe for UB (what happens if NULL is
defined to (void *)1?).
{
if(str==' ')
{ st[j++]=st[i++];

while(str==' ') i++;

}
else
st[j++]=str[i++];


Bad logic. This will not do what you expect it to do.
Your string "test string" will become "teststring".
}
st[j]=NULL;

Again, it's safer to use \0.
strcpy(str,st);
puts(str);

This is a waste of cpu cycles. Just print out st instead and save the
overhead of the unneeded function call.
}

(compiled in turbo c++ for windows version 3.1)
i know there should be other ways to write this program (thats a bad
program ),can someone help me in writing that program in compact way
and give me some logic and tips.
i want to use gcc but i don`t have linux operating system and one
software which allows gcc compiling in windows (cygwin) is not working.
anyone can advice me such a program that allows compiling with gcc in
windows( this is off topic but advices are always welcome).

thankx

Here is a modified version of your program that should perform better
than the one above. I've tested this however there is always room for
human error so be forewarned.

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

int main(void)
{
char str[20], st[20];
int i = 0, j = 0;
size_t len = 0;

printf("enter string: ");
fflush(stdout);

/* note that our string will also have
* a newline character at the end of it */
fgets(str, 20, stdin);
len = strlen(str);

/* comment this out if you want to retain the
* newline */
if (str[len - 1] == '\n')
{
str[len - 1] = '\0';
len--;
}

for (i = 0; i < len; i++)
{
/* if the next two characters are spaces then
* we need to condense them into one */
if (str == ' ' && str[i + 1] == ' ')
{
st[j++] = ' '; /* we want to keep one space */
while (str == ' ')
{
i++;
}
}

st[j++] = str;
}

st[j] = '\0';

printf("%s\n", st);

return(0);
}
 
A

ash

thnankx joe estock, i really like your code.Its wonderful to see "many
logics to solve one question". can u suggest how to develope logics
faster?
 
J

Joe Estock

ash said:
thnankx joe estock, i really like your code.Its wonderful to see "many
logics to solve one question". can u suggest how to develope logics
faster?

What code? Without context I have no idea what you are talking about. >
Please include enough of the previous post in your reply so that
everyone can follow the discussion. It's by chance that I checked this >
thread again today. For those of you who do not have access to the
previous thread I have included it's context below.
> Sounds simple enough. The portable solution would be to encase this
> functionality into a function so that you can easily utilize it among
> other utilities, but for now I'll keep it simple.

[snip code from OP]
>> {
>> if(str==' ')
>> { st[j++]=st[i++];
>>
>> while(str==' ') i++;
>>
>> }
>> else
>> st[j++]=str[i++];

> Bad logic. This will not do what you expect it to do.
> Your string "test string" will become "teststring".

[snip some of my code]
> for (i = 0; i < len; i++)
> {
> /* if the next two characters are spaces then
> * we need to condense them into one */
> if (str == ' ' && str[i + 1] == ' ')
> {
> st[j++] = ' '; /* we want to keep one space */
> while (str == ' ')
> {
> i++;
> }
> }
>
> st[j++] = str;
> }


I do not fully understand your question "develop logics faster". As per
my original reply I was stating that the logic in your code was
incorrect. In this context logic means the "logical series of events and
results". You wanted to remove extra spaces (in essence you wanted to
condense multiple spaces into one single space). Your original code was
removing *all* spaces.

The only way I can think of to explain logic is by example. Consider the
following line:

10,20,30,40\n

Let's presume that this line is from a file that contains several other
lines like it. We want to read each line in this file and print the
information in a more human-readable format. The logic of our program
would be something like the following:

Open the file for reading.
Ensure open was successful.
Print the column header.
Read a line from the file.
While there are lines left in the file...
Separate the values by splitting the string on ",".
Print out the values.
Read another line from the file.
End while.
Close file.
Return status.
 
C

CBFalconer

ash said:
thnankx joe estock, i really like your code.Its wonderful to see "many
logics to solve one question". can u suggest how to develope logics
faster?

Please don't use silly abbreviations like 'u'. Please do include
adequate context.

A blank suppression routine can be much simpler, if I correctly
recall the subject in the absence of context and the silly changing
of the subject line.

#include <stdio.h>
#define TAB 9 /* for Ascii only */

int main(void) {
int ch, inword;

inword = 0;
while (EOF != (ch = getchar()))
if ((' ' == ch) || (TAB == ch)) {
if (inword) putchar(' ');
inword = 0;
}
else {
putchar(ch);
inword = 1;
}
return 0;
} /* untested */

use via "progname <source [>dest]" on most systems.
 
L

lovecreatesbeauty

Peter said:
lovecreatesbeauty wrote:
What if the original string is "". Is that an error? If so, why?

The string literal "" just includes a delimiter of null terminator, its
string length is 0.
int spacerm(char *dst, char *src)
{
said:
if (src == 0 || dst == 0)
Why bother? [Rehtorical]

If both src and dst are pointers have null values and caller pass these
null pointers into the function, then code inside the function like
this: *dst++ = *src++; will cause undefined behavior. Doing nothing is
better than causing an undefined behavior in my code, I feel. I still
want to listen to you. Thank you for providing comments on my code,
it's helpful to me.

(Joe Estock's code is better than mine, I think it should be an
expert's piece.)

lovecreatesbeauty
 
F

Frederick Gotham

lovecreatesbeauty posted:

If both src and dst are pointers have null values and caller pass these
null pointers into the function, then code inside the function like
this: *dst++ = *src++; will cause undefined behavior.


You have to decide which part of your code is responsible for checking
for "errors". I myself thrive on writing efficient code (I find it
exciting and enjoyable), so if I were to write a function to convert a
decimal digit to an actual integer, I would write:

(I believe a character literal is of type "int" in C... is that right?)

unsigned DigitToInt( int const c )
{
return c - '0';
}


I certainly wouldn't write:

int DigitToInt( int const c )
{
if( c < '0' || c > '9' ) return -1;

return c - '0';
}

I would "delegate the responsibility" to the caller function to make sure
that everything is in order. Nonetheless, asserts are free, so the
finished product might be something like:

inline unsigned DigitToInt( int const c )
{
assert( c >= '0' && c <= '9' );


return c - '0';
}



When I'm writing functions which deal with strings, I never check for
null pointers -- I delegate that responsibility to the calling function.
 
T

Tom St Denis

Frederick said:
You have to decide which part of your code is responsible for checking
for "errors". I myself thrive on writing efficient code (I find it
exciting and enjoyable), so if I were to write a function to convert a
decimal digit to an actual integer, I would write:

What you call efficient I just call lazy.
(I believe a character literal is of type "int" in C... is that right?)

unsigned DigitToInt( int const c )
{
return c - '0';
}


I certainly wouldn't write:

int DigitToInt( int const c )
{
if( c < '0' || c > '9' ) return -1;

return c - '0';
}

I would "delegate the responsibility" to the caller function to make sure
that everything is in order. Nonetheless, asserts are free, so the
finished product might be something like:

And you'll never write robust code. Trust me, people will pass stupid
things to your functions.

Sure you can't catch all errors but simple checks for NULL or invalid
symbols is a smart thing to do. It also makes debugging simpler when
you trap errors.
inline unsigned DigitToInt( int const c )
{
assert( c >= '0' && c <= '9' );


return c - '0';
}

Not only is this not portable but in 2006 the use of "inline" is
redundant for functions like this. The compiler is smart enough to
figure it out.
When I'm writing functions which deal with strings, I never check for
null pointers -- I delegate that responsibility to the calling function.

Again, you also don't write robust code.
From my experience from my projects and the support email I get, most
people who can't get functions to work don't read the manual. A good
portion of the others are weak C programmers and don't understand
pointers and the like fully. A simple != NULL or other check may seem
to kill performance [hint: they don't] but are totally worth it.

Tom
 
F

Frederick Gotham

Tom St Denis posted:
What you call efficient I just call lazy.


Think clearly. If I was really that lazy I wouldn't be on this newsgroup
writing code which provides me with no monetary gain.

And you'll never write robust code. Trust me, people will pass
stupid things to your functions.


Then "people" may expect undefined behaviour. I don't write functions to
be used by three year olds.


Not only is this not portable but in 2006 the use of "inline" is
redundant for functions like this. The compiler is smart enough to
figure it out.


I'm not sure if the same applies for C, but in C++, the decimal digit
characters are guaranteed to be sequential and in ascending order.

Can someone please clarify if this holds true for C?

Again, you also don't write robust code.


I do... but it's within the calling function:

#include<...

int main(void)
{
int c = 'r';

if ( isdigit(c) ) DigitToInt(c);
}


Or, wherever possible, I write my code so that it simply can't be
erroneous, for example:


int main(void)
{
int i = 54;

int j = i % 3;

FunctionThatWantsNumberBetweenZeroAndTwoInclusive( j );
}

From my experience from my projects and the support email I get, most
people who can't get functions to work don't read the manual. A good
portion of the others are weak C programmers and don't understand
pointers and the like fully. A simple != NULL or other check may seem
to kill performance [hint: they don't] but are totally worth it.


Yes, in the calling function:

if (p) DoSomething( p );
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,763
Messages
2,569,563
Members
45,039
Latest member
CasimiraVa

Latest Threads

Top