Having problem to do appropriately

Question

1.00/5 (1 vote)

See more:

Question
Create a class AlphaString that stores only alphabets in a string of arbitrary length. The string has a constructor that makes sure that String has only Alphabets. The class also has a copy constructor, and an overloaded + operator. The overloaded + operator should perform the concatenation of two strings.

#include <string.h>
#include<iostream.h>
#include<conio.h>
using namespace std;

class AlphaString {
public:
   AlphaString( char *ch );  // Declare constructor
   ~AlphaString();           //  and destructor.
private:
   char    *_text;
   size_t  sizeOfText;
};

AlphaString::AlphaString( char *ch ) {
    int i=1;
    int k=int(ch[i]);
    int flag=0;
    while(strlen(ch))
    {
        if((k>=65 && k<=90) || (k>=97 && k<=122))
           flag =0;
        else
        {
           flag =1;
           break;
        }
        k=int(ch[i]);
        i++; 
    }
    sizeOfText = strlen( ch ) + 1;
    if(flag==0)
    {
        // Dynamically allocate the correct amount of memory.
        _text = new char[ sizeOfText ];
    }
}

// Define the destructor.
AlphaString::~AlphaString() {
   // Deallocate the memory that was previously reserved
   //  for this string.
   if (_text)
        delete[] _text;
   }
   
// Copy Constructor.
AlphaString::AlphaString(AlphaString & objB) {
}
int main() {
    char str[25];
    cout<<"Enter First String: ";
    cin>>str;
    AlphaString ob1(str);
    cout<<"Enter Second String: ";
    cin>>str;
    AlphaString ob2(str);
    cout<<ob1._text+ob2._text;
    AlphaString ob3(ob1);
    
    getch();
    return 0;
}

[edit]Code block added to preserve formatting - OriginalGriff[/edit]
[edit]Corrected code alignment and balanced braces - Andrew Brock[/edit]

Posted 26-Feb-11 5:41am

RinkuRajibSaha

Updated 26-Feb-11 18:16pm

Andrew Brock

v3

Add a Solution

Comments

OriginalGriff 26-Feb-11 11:43am

And your question is?

3 solutions

Solution 3

AlphaString::AlphaString( char *ch ) { //You are not changing the contents of ch, so why not make it const?
    int i=1; //All array indices start at 0 in C++. Strings included.
    int k=int(ch[i]); //This is the 2nd character in the string
    int flag=0; //You should use meaningful variable names, like onlyAlpha
    //while(strlen(ch)) is just horrid.
    //The length of the string never changes, so you might as well have while(1)
    //Furthermore, strlen works by iterating over every character in the string to find the termination character. This is happening every loop.
    while(strlen(ch))
    {
        if((k>=65 && k<=90) || (k>=97 && k<=122)) //Why not use characters (as Richard MacCutchan suggested)
           flag =0; //flag is already 0. Why are you setting it again?
        else
        {
           flag =1;
           break;
        }
        k=int(ch[i]); 
        i++;
    }
    sizeOfText = strlen( ch ) + 1;
    if(flag==0)
    {
        // Dynamically allocate the correct amount of memory.
        _text = new char[ sizeOfText ];
        //What are the contents of _text? It is not the string.
    }
    //What happens to _text if flag != 0?
    //It remains uninitialised, and likely does not hold the value NULL.
    //This will cause problems with the destructor.

A corrected solution:

AlphaString::AlphaString(const char *ch) : _text(0), sizeOfText(0) {
    int i = 0;
    bool bOnlyAlpha = true; //You should use meaningful variable names, like onlyAlpha
    while(ch[i] != 0) { //0 is the null terminator
        if((ch[i]<'A' || ch[i]>'Z') && (ch[i]<'a' || ch[i]>'z'))
           flag =0; //flag is already 0. Why are you setting it again?
           bOnlyAlpha = false;
           break;
        }
        ++i;
    }
    sizeOfText = i + 1; //If you are paying attention, you will notice that i holds the length of the string, not including the terminator
    if (bOnlyAlpha) {
        // Dynamically allocate the correct amount of memory.
        _text = new char[sizeOfText];
        memcpy(_text, ch, sizeOfText); //Could use strcpy, but memcpy is faster
    }
}

Posted 26-Feb-11 20:24pm

Andrew Brock

Solution 2

I think you want something like this:

#include<ctype.h>

class AlphaString 
{
public:   
	AlphaString( char *ch );  
	// Declare constructor   
	~AlphaString();           
	//  and destructor.
private:   
	char    *_text;   
	size_t  sizeOfText;
};
AlphaString::AlphaString( char *ch ) 
	: _text(0),sizeOfText(0) 
// Initialize to make sure they don't contain garbage, 
// as required by your destructor
{    
	if(ch)
	{
		size_t numberOfCharacters = 0;
		char* ptr = ch;
		while(*ptr) // figure out the required memory size 
		{
			char c = *ptr;
			if(isalpha(c))
			{
				numberOfCharacters++;
			}
			ptr++;
		}
		if(numberOfCharacters)
		{
			_text = new char[numberOfCharacters+1];
			
			char* ptrSource = ch;
			char* ptrDestination = _text;
			while(*ptrSource) // copy the alphbetic characters
			{
				char c = *ptrSource;
				if(isalpha(c))
				{
					*ptrDestination = c;
					ptrDestination++;
				}
				ptrSource++;
			}
			_text[numberOfCharacters] = '\x0'; // zero terminate the string
			sizeOfText = numberOfCharacters;
		}
	}
}

This should work :) - you can use your own destructor implementation ...

This constructor addresses a few important points:
1. Initialization of member variables:
_text(0),sizeOfText(0)
C++ does not initialize member variables, that’s something you have to perform yourself.

2. if(ch) tests if ch is not NULL – if it is, the original code is in trouble

3. count the number of alphabetic characters

4. if there are any alphabetic characters - allocate the required amount of memory:
number of alphabetic characters + 1 (for terminating zero)

5. copies only the alphabetic characters to the allocated buffer

6. zero terminates the buffer

Regards
Espen Harlinn

Posted 26-Feb-11 6:45am

Espen Harlinn

Updated 26-Feb-11 23:25pm

v4

Comments

Sergey Alexandrovich Kryukov 26-Feb-11 19:11pm

Oh, we're experiencing "mass destruction" down-voting here...
My 5.
--SA

Espen Harlinn 2-Mar-11 6:42am

Thank you, SAKryukov

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

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

Richard MacCutchan · Accepted Answer · 2011-02-26T05:55:00

Solution 1

if((k>=65 && k<=90) || (k>=97 && k<=122))

Why not make it readable by coding like:

if((k >= 'A' && k <= 'Z') || (k >= 'a' && k <= 'z'))

Posted 26-Feb-11 5:55am

Richard MacCutchan

Comments

Espen Harlinn 26-Feb-11 12:55pm

It may make it more readable, but it's still going to crash and burn :)

Richard MacCutchan 26-Feb-11 12:57pm

Get the simple things right and the rest will follow.

Espen Harlinn 26-Feb-11 13:03pm

Right - seem to remember a guy, a few years back, who was taking about "weapons of mass destruction", but then he didn't get anything right - but he sure got a huge following for a few years, including a certain prime minister :)

Sergey Alexandrovich Kryukov 26-Feb-11 19:09pm

Good point, my 5.
Also, I disagree with somebody who seemingly started "mass destruction" down-voting around, so I feel I want to compensate.
--SA

Albert Holguin 26-Feb-11 21:01pm

Good point... but not really a solution, should probably be a comment. (voted 4 since its still very good feedback)

Richard MacCutchan 27-Feb-11 3:33am

I agree, but the code block (pre tags) does not work within comments, and I wanted to highlight my suggestion.

Albert Holguin 27-Feb-11 14:13pm

that's true... may be a good suggestion for code blocks to work in comments as well (for webmaster)

Richard MacCutchan 28-Feb-11 4:04am

I think it has already been reported.