-1
#include <iostream>
#include<cstring>

using namespace std;
class String
{
    private:
    char *sptr;

    public:
    String()
    {

     }

    String(char str[])
    {
      sptr = new char[strlen(str) + 1];
      strcpy( sptr, str );
    }

    String(const String& source)
    {

        sptr = new char[strlen(source.sptr) + 1];
        strcpy( sptr, source.sptr);

    }

    ~String()
    {
      delete sptr; 
    }

    String operator=( const String& other )
    {

        if(&other!=NULL)
        {

            String tmp( other );   
            sptr = new char[strlen(tmp.sptr) + 1];
            strcpy( sptr, tmp.sptr);
        }
        return *this;    
    }
    void display()
    {

      for( char const* p = sptr; *p != '\0'; ++ p) {
            std::cout << *p;
        }
        cout<<endl;

    }


};

 int main()
 {

   String a1;
   String a2("Hello ");   
   String a3(a2);

   a2.display();
   a3.display();

    //a2.change("Java ");
    a2.display();
    a3.display();
}

The Output pf the program is

Hello Hello Hello Hello.

But I am undone to do the following changes in my code... i.e.. In Main () I created the following objects

String a1;
String a2("Hello ");   
String a3(a2);

a2.display();
a3.display();

//a2.change("Java ");
a2.display();
a3.display();

I want to change the the object a2(commented) with Java and desired the output as

Hello Hello Java Hello 

through deep copy of the Data members through this pointers.

How can I change the Hello string to java

Akash
  • 425
  • 2
  • 7
  • 21
  • I dynamically allocated a char array along with their size – Akash Sep 13 '18 at 00:57
  • 2
    You don't dynamically allocate anything in the default constructor. As such, any default-constructed instance of this object will result in undefined behavior when `delete`ing the uninitialized pointer in its destructor, and a guaranteed crash. Now, exactly what is your specific C++ question? "I want to change the object" something or other is not a question. – Sam Varshavchik Sep 13 '18 at 01:00
  • `if(&other!=NULL)` It's safe to assume that this condition is always `true`. If it would be `false`, you would already be in undefined behavior, so it's safe to assume it's `true` anyway. – François Andrieux Sep 13 '18 at 01:01
  • Your assignment operator neglects to `delete` the previously allocated buffer. It will leaks memory. – François Andrieux Sep 13 '18 at 01:03
  • @ François Andrieux help to me resolve out to this .. – Akash Sep 13 '18 at 01:13
  • @AkkiêThakur Your `String::operator =` can be written in 3 lines `{ String temp(other); std::swap(temp.sptr, sptr); return *this; }`. Also, your `delete sptr` should be `delete [] sptr;` – PaulMcKenzie Sep 13 '18 at 01:14
  • @AkkiêThakur -- This 1 line, full, `main()` program using your current code causes issues: `int main() { String s; }`. It was already mentioned by SamVarshavchik what the issue is. – PaulMcKenzie Sep 13 '18 at 01:17
  • i am comes over out to the problem if(&other!=NULL) but i am unable to change the string a2 with "Java" – Akash Sep 13 '18 at 01:18
  • @AkkiêThakur -- Why are you trying to use a faulty `String` class in the program? First fix all of the faults already mentioned in the `String` class. You can't build a program on a faulty foundation. – PaulMcKenzie Sep 13 '18 at 01:19
  • @ PaulMcKenzie Does I need to change the class name??? – Akash Sep 13 '18 at 01:21
  • Your `String` class itself cannot be used to write a working program. It has many bugs. I don't know how much clearer to say this. Those bugs have been mentioned already. – PaulMcKenzie Sep 13 '18 at 01:22
  • @Sam Varshavchik Through deep copy i want to change a2 with "Java"..... – Akash Sep 13 '18 at 01:27
  • @AkkiêThakur -- Forget about the "Java" string. The problem happens before that -- your `String` class is no good and you need to fix it. I hate to make it more blunt, but it sounds like you're not taking any advice. – PaulMcKenzie Sep 13 '18 at 01:29
  • @ PaulMcKenzie no sir I will fix some of the bugs gently... – Akash Sep 13 '18 at 01:32
  • but i am asking you it will helpfull to me if i change the class name string to demo – Akash Sep 13 '18 at 01:33
  • @AkkiêThakur -- [Look at this example of your code if you need proof](https://www.ideone.com/cqPp6f). So are you now going to believe SamV and myself about your `String` class being faulty on the simplest of operations, i.e. default construction? How can you now build a program using a `String` class that can't even do the simplest things correctly? Also what does changing the name of the class have anything to do with how the class operates? You can change it to "YabbaDabbaDo" and it will still be faulty. – PaulMcKenzie Sep 13 '18 at 01:33
  • @ PaulMcKenzie sir i understand that. But sir this is my assignment in which i used the class name String ... so sir i tried many ways and everytime i got some errors at last i am asking you to please review my code and provide me some edition in the code. – Akash Sep 13 '18 at 01:39
  • https://stackoverflow.com/users/3363978/praanj Sir please check it out this problem – Akash Sep 13 '18 at 01:48
  • What is `change` supposed to do? Why not just state `a2 = "Java ";` if all you want to do is change `a2` to now be `"Java "`? But before you do that, you need to read my answer below, as your assignment operator is not correct. – PaulMcKenzie Sep 13 '18 at 02:08

1 Answers1

2

Your String class cannot be used to build a program upon, due to the faults within the String class. You must fix those errors first before even thinking about using String as a string class.

Therefore this answer addresses how to fix String first -- from there, you should now be able to write the program knowing you're not working on a faulty foundation.


Error 1: Default construction

You failed to initialize sptr when a String is default constructed. Thus when ~String() is executed, the call to delete will attempt to delete an uninitialized pointer.

Simple 1 line main functions such as this one:

int main()
{
   String s;
} // <-- destructor of s may crash

will show undefined behavior and may crash, as shown by this example.

To fix this:

class String
{
   public:
     String() : sptr(nullptr) {}
   //...
};

Now when delete [] is issued on sptr, no harm will occur since it is perfectly valid to delete [] a nullptr (it becomes basically a no-op).


Error 2: Faulty assignment operator

Your String::operator= fails to deallocate the previous memory that was allocated. In addition, the check for NULL is incorrect, since &other will always be true.

I am assuming this is the check for self-assignment, but it is not written correctly. The check for self-assignment is done by comparing the address of the current object (this) with the address of the object being passed in:

if (this != &other)

Once you have that, then you could have written the rest of the function this way:

if(this != &other)
{
    String tmp( other );   
    std::swap(tmp.sptr, sptr);
}
return *this;

All this function does is copy other to a temporary String called tmp, swap out the sptr with the tmp.sptr and let tmp die off with the old data. This is called the copy / swap idiom, where you're just swapping out the contents of the current object with the contents of the temporary object, and letting the temporary object die off with the old contents.

Note that the check for self-assignment isn't necessary when using copy / swap, but there is no harm done in making the check (could even be a small optimization done when the check for self-assignment is present).

Edit: The other issue is that operator= should return a reference to the current object, not a brand new String object. The signature should be:

String& operator=( const String& other ) // <-- note the return type is String&
{
  //... code
  //...
  return *this;
}

Error 3: Wrong form of delete

Since you allocated using new [], you should be using delete [], not just delete in the destructor:

~String() 
{ 
   delete [] sptr;
}

Given all of these changes, the following code no longer has issues, as shown by the Live Example here.

Now that you have a working String class, then you can build your application from there.


As to your program, you can easily change your string by using the assignment operator. There is no need for a change() function:

String a2("Hello ");   
a2.display();
a2 = "Java ";
a2.display();

Since String::operator=(const String&) is no longer faulty with the changes above, assignment can now be done without issues.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • 1
    Another one: `operator=` shouldn't return by value. – Ben Voigt Sep 13 '18 at 02:40
  • Yes, I will add that to the list. – PaulMcKenzie Sep 13 '18 at 02:40
  • Thanks sir I will try it with learning all your provided concepts – Akash Sep 13 '18 at 05:59
  • @PaulMcKenzie your code shows a error that nullptr was not declared in the scope.. class String { public: String() : sptr(nullptr) {} //... }; but when i removed null ptr from command String() : sptr(nullptr) {} it works but the null should be assigned ........ what the logic behind that – Akash Sep 13 '18 at 10:27
  • What version of C++ are you using? You see in the online example the code compiles with no issues. The `nullptr` is a C++ 11 keyword -- are you using a C++ 11 compliant compiler? – PaulMcKenzie Sep 13 '18 at 12:02
  • Also, as a replacement, you could have simply done this: `String() : sptr(0) {}` – PaulMcKenzie Sep 13 '18 at 12:54