Click here to Skip to main content
16,017,788 members
Please Sign up or sign in to vote.
3.00/5 (2 votes)
See more:
C++
#include "stdafx.h"
#include <iostream>
#include <fstream>
#include <conio.h>
#include <string>
#include <stdio.h>
#include <stdlib.h>
#include <map>

using namespace std;

int jomleTokalame(string *a,string c,int count){
    string cc="";
    count =0;
    for(int i = 0 ; c[i]!='\0';i++){
        cc+=c[i];
        if (c[i]==' ' || c[i+1]=='\0'){
            a[count]=cc;
            count++;
            cc="";
        }
    }
    return count;
}

int _tmain(int argc, _TCHAR* argv[])
{
    int location[100];
    string str[100];
    string a[100];
    string s;
    int n,i=1,j,k=0,g;
    cout<<"plz enter number of lines:\n";
    cin>>n;
    for(i=1;i<=n;i++){
        getline(cin,a[i],'\n');
        s=a[i];
        g=jomleTokalame(str,s,0);
        for(j=0;j<=g;j++)
            cout<<str[j];
    }
    getch();
    return 0;
}
Posted
Updated 12-Feb-11 7:31am
v2
Comments
TweakBird 12-Feb-11 13:34pm    
What is your problem. Did you get any error or exception?. P.S: Please use 'Improve question' and edit the question add some more details like (error or exception or if stuck, where it is ?).

This code is beyond repair, almost every line makes no sense of shows deep confusion about language and/or programming. All code should be removed and clear code written.

Let's start:

There is not need to mix up std includes with C-stile I/O includes; std is more then enough for the purpose.

The function jomleTokalame is not needed and makes no sense, it serves no purpose, its name look cryptic rather then descriptive.
The value of parameter count of this function is not used (assigned to 0, in call also assigned with 0 which is not used), should be replaced with local variable. (However, whole calculation is not needed because class the length in known via string.length().) Passing the first string parameter via pointer is bad, passing a parameter by reference is more adequate (however, it makes no sense, because function is of now use; the strings can be assigned). Biggest mistake in this function is using null character. The type string does not use this technique (which is itself extremely bad).

In the function _tmain, variable location is not used, other parameters make no sense, because there is not an array which elements are assigned to the strings read from the stream. The outer loop in this function skips a[0] as if it used 1-based array addressing. The function getch is deprecated…

I don't think I need to continue.

What should be done?

Use type vector<string></string> (include <vector>), initialize its instance to the length of n, write a loop 0 to n-1, use a loop variable as a vector index; in the loop, read a string and assign its value to the array element indexed by the loop variable. It all will take some 10 lines of code if not less. That's it.

—SA
 
Share this answer
 
v8
Comments
Nish Nishant 12-Feb-11 22:33pm    
Excellent reply. My 5.
Sergey Alexandrovich Kryukov 12-Feb-11 23:12pm    
Thank you. Some typo fixed, text clarified.
--SA
Sandeep Mewara 13-Feb-11 1:32am    
Deserves 5++ :)
Sergey Alexandrovich Kryukov 13-Feb-11 2:03am    
Thank you.
--SA
Harrison H 13-Feb-11 16:02pm    
This is the kind of discourse the OP needed. My 5
Let me start by pointing out what is wrong with your code:
#include "stdafx.h"
#include <iostream>
#include <fstream> //this is for file I/O. you aren't using files.
#include <conio.h>
#include <string>
#include <stdio.h> //You don't need this. you are using std:: functions instead for printing and reading to the console.
#include <stdlib.h> //You dont need this either. not sure why you thought you did.
#include <map> //You arent using the std::map. Why are you including this?

//be careful when using "using namespace". This can only be used to use a single namespace.
//you should instead use:
//using std::cin;
//using std::cout;
//...
using namespace std;

//When passing in the parameter c another copy is made of the string.
//You are creating 2 copies of the string when you only need the original.
//Why are you passing in count. The first thing you do is set a copy of the value to 0.
int jomleTokalame(string *a,string c,int count) {
    string cc="";
    count =0;
    for(int i = 0 ; c[i]!='\0';i++){
        cc+=c[i];
        if (c[i]==' ' || c[i+1]=='\0'){
            a[count]=cc; //Yet another copy of a string.
            count++;
            cc="";
        }
    }
    return count;
}

//here you are using the typed main function, which will use whichever character set you are compiling with.
//All your std::strings are are ASCII
int _tmain(int argc, _TCHAR* argv[]) {
    //You have limited space on the stack.
    //You should avoid putting large arrays on the stack, or arrays of large data types.
    int location[100]; //unused variable taking 400 bytes on the stack. Use it or get rid of it
    string str[100]; //Each std::string typically takes 12 bytes. This is taking 1200 bytes on the stack
    string a[100]; //again, this is taking 1200 bytes on the stack
    string s;
    int n,i=1,j,k=0,g; //You should change more meaningful variable names. Unused variable k. Initial value of i is never used.
    cout<<"plz enter number of lines:\n";
    cin>>n; //what if the user enters 
    for(i=1;i<=n;i++) { //I think you are wanting to start with for(i=0;i<n;i++).
        //Array indices start at 0 and go to N-1.
        getline(cin,a[i],'\n');
        s=a[i]; //Note: this COPIES the value of a[i]. This is slow and unnecessary.
        g=jomleTokalame(str,s,0);
        for(j=0;j<=g;j++)
            cout<<str[j];
    }
    getch(); //This function is deprecated, you should instead use _getch();
    return 0;
}


Now that you know what is wrong. Lets look at the same solution that is much better:
#include <iostream>
#include <conio.h>
#include <string>

using std::cout;
using std::cin;
using std::string;

//Now we will pass parameter strLine by const reference
//This does not create a copy, but does say that we can't change the original
int jomleTokalame(string *strWords, const string &strLine) {
    //instead of using a variable cc, we will just modify the array directly. This will avoid copying.
    int nWords = 0;
    strWords[nWords]="";
    for(int nChar = 0; strLine[nChar] != '\0'; ++nChar){//++X is often (marginally) faster than X++
        strWords[nWords] += strLine[nChar];
        if (strLine[nChar] == ' ' || strLine[nChar + 1] == '\0'){
            //no copying to be done now.
            strWords[++nWords]=""; //Make sure the next string is empty.
        }
    }
    return nWords + 1; //This is a better place to increment the number of words
}

int _tmain(int argc, _TCHAR* argv[]) {
    string strWords[100]; //you should really use something like a std::vector for this, but i will let you do that.
    string *strLines = NULL; //This will be allocated dynamically once we know how big to make the array.
    int nLines, nLine, nWords, nWord;
    cout << "plz enter number of lines:\n";
    cin >> nLines;
    strLines = new string[nLines]; //This way you won't get a buffer overflow if nLines>100 and it doesn't take up precious space on the stack.
    for (nLine = 0; nLine < nLines; ++nLine) {//++X is often (marginally) faster than X++
        getline(cin, strLines[nLine], '\n');
        nWords = jomleTokalame(strWords, strLines[nLine]); //pass a[nLine] in directly
        for (nWord = 0; nWord < nWords; ++nWord)
            cout << strWords[nWord];
    }
    delete[] strLines; //delete the dynamically allocated array of strings
    _getch();
    return 0;
}
 
Share this answer
 
v2
Comments
Sandeep Mewara 13-Feb-11 1:33am    
Well resolved... good answer!
Emilio Garavaglia 13-Feb-11 15:38pm    
- edited -
leaks memory:
strLines = new string[nLines]; is never deleted.
A delete[] strlines; is needed before _getch();
Andrew Brock 14-Feb-11 10:52am    
oops... good catch. I will resolve the code for reference.
Emilio Garavaglia 14-Feb-11 11:03am    
already done by me last night
Andrew Brock 14-Feb-11 11:06am    
Yes, I noticed after posting the comment. I tried to edit my comment but the changes weren't saved.
Thanks.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

  Print Answers RSS
Top Experts
Last 24hrsThis month


CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900