can this small code be improved !!

**samirself** · 04-30-2005

this code just circularly shifts the given string,
n(=4) is the no. of charactars to be shifted

input: sab ulta ho gya
output: gyasab ulta ho

now, can there any refinements !!!!!!!!
suggestions or changes wd be appreciated.

thanks

Code:

#include<iostream.h>
#include<conio.h>
#include<string.h>
void main()
{
clrscr();
char *s="sab ulta ho gya";
cout<<s;
char p;
int n=4,i=0,k=0;
int len;
len=strlen(s);

while(i<n) {
      k=0;
      p=s[len-1];
      while(len-1>0)
      {  s[len-1]=s[len-2];   
           len--; 
      };
      s[k]=p;
     i++;
     len=strlen(s);
 };
cout<<"\n"<<s;
getch();
}

**Thantos** · 04-30-2005

Yes that code needs lots of improvements.

I'd share some but whats the point? You won't listen and I know it.

**Salem** · 04-30-2005

> #include<iostream.h>
Modern C++ just has
#include <iostream>

> #include<conio.h>
Only old DOS compilers can be sure of having anything like this. To everyone else, it's useless.

> void main()
Very bad - main returns int, always has done, always will do.

> clrscr();
Ugh - more non-standard features. Next to making programs go "beep", newbies seem to like clearing the screen for some reason. Useful programs don't bother, mostly because it would become extremely annoying very quickly.

> char *s="sab ulta ho gya";
Given your attempts at s[len-1]=s[len-2], this code would fail horribly on machine which store string constants in read only memory. If you really want to change the string "in place", then use an array
char s[] = "sab ulta ho gya";

> cout<<s;
Minor point, but you didn't flush the output.

Code:

while(i<n) {
      k=0;
      p=s[len-1];
      while(len-1>0)
      {  s[len-1]=s[len-2];   
           len--; 
      };
      s[k]=p;
     i++;
     len=strlen(s);
 };

So rather than move the string 'n' places at once, you move it one char at a time.
This is going to be horribly expensive on long strings.
Not to mention that you keep recalculating the length of the string (it should be a constant), there should only be one call to strlen.

Also, all those ; following a brace are unnecessary.

Useful string manipulation functions don't modify the string "in place". Something like
void rotate ( char *dest, const char *src, int places );
would be a far more flexible approach.

**samirself** · 05-01-2005

Thantos dear, i wld love to read and accept ur comments

and thank u Salem for replying.

**samirself** · 05-01-2005

So rather than move the string 'n' places at once, you move it one char at a time.

i think i am playing with one character at a time..............

and what if main() returns nothing !!

**Quantum1024** · 05-01-2005

Originally Posted by samirself

i think i am playing with one character at a time..............

Thats what your code dose at the moment.

Originally Posted by samirself

and what if main() returns nothing !!

Main SHOULD return zero to indicate the program exited without error and none-zero to indicate an error. Using void main causes mains return value to be undefined.

**samirself** · 05-01-2005

currently i am using Turbo C++ Version 3.0, salem comments hint that i shd upgrade
so what shd i do...........

**misplaced** · 05-01-2005

it's not exactly your compiler....a lot of c/c++ compilers will let you get away with a lot of stuff. i don't know if turbo c++ is a command line compiler or not. either why, invoke the compiler with the switch "-ansi" to ensure you only use standard c++

**samirself** · 05-01-2005

invoke the compiler with the switch "-ansi"

?????????????????sorry but how?

**sean** · 05-01-2005

To do that you'll have to start the compiler from the command-line. If I recall, the executable for TC++ 3.0 is tcc.exe, so go to the command line, navigate to the "bin" directory, and type, "tcc -ansi"

**hk_mp5kpdw** · 05-01-2005

This program only needs to be a few lines if you make use of the what's already available to you in the STL:

Code:

#include <iostream>
#include <string>
#include <algorithm>

using namespace std;

int main()
{
    string str = "sab ulta ho gya";

    cout << str << endl;
    rotate(str.begin(),?????,str.end());
    cout << str << endl;

    return 0;
}

You figure out what needs to take the place of the ????? and you will have another solution.

**samirself** · 05-03-2005

i tried this on Dev-C++

rotate(str.begin(),?????,str.end());

and

error:syntax error before ? token

rotate(str.begin(),"gya",str.end());

error:12 no matching function for call to `rotate(__gnu_cxx::__normal_iterator<char*
correct it sir...

**hk_mp5kpdw** · 05-03-2005

Well, you need to find the correct bit of code to put in place of the ????? as I indicated. Hint: It has to do with an iterator.

Thread: can this small code be improved !!

Thread Tools

Search Thread

Display

can this small code be improved !!

Similar Threads

Need some help with this small code

Binary Search Trees Part III

True ASM vs. Fake ASM ????

Interface Question

Can you have nested code block? What does the compiler do? For example ...