Click here to Skip to main content
Rate this: bad
good
Please Sign up or sign in to vote.
See more: C++
// In HelloWorld.h

#include <string>
using std::string;
 
class HelloWorld {
 
public:
  HelloWorld();
  string getText();
  void setText(string);
  virtual ~HelloWorld();
 
private:
  string* text;
 
};
 
// In HelloWorld.cpp
#include "HelloWorld.h"

// Constructors
HelloWorld::HelloWorld() {
  text = new string("HelloWorld");
}
 
// Accessors
string HelloWorld::getText() {
  return *text;
}
 
// Mutators
void HelloWorld::setText(string text) {
  *(this->text) = text;
  return;
}
 
// Destructor
HelloWorld::~HelloWorld() {
  delete text;
}
Posted 15-Apr-13 22:03pm
Edited 16-Apr-13 10:24am
v4
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 1

class HelloWorld {
 
public:
  HelloWorld();
  string getText() const { return text;}
  void setText(const string &s) { text = s;}
  virtual ~HelloWorld();
 
private:
  string text;
 
};
 
//Constructors
HelloWorld::HelloWorld() {
  text = string("HelloWorld");
}
 
It's common practice to define get/set methods inline. Also, don't define std::string as pointer. If you really want a pointer, use the c_str() method of std::string.
  Permalink  
v3
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 2

Let's start with the text member. Why have you declared it as pointer? If no other reasons speak against it, just declare it as
     string text;
And thereby you don't have to care about its allocation and deletion.
 
The getText accessor is in principal fine. Here I would just consider two things:
- not all programs do string handling with the std::string class; it would be more general to just return a char* (or wchar_t*) to a null-terminated string. In that case you would be using the c_str function.
- why not make the access function const and declare thereby that it's not modifying the base object?
 
The setText setter function is ok. When you declart text as string (and not as a pointer) it will look a lot simpler:
void HelloWorld::setText (const string& t)
    {text = t;}
 
Note that I made the argument a const reference, which avoids copying the string to a temp variable.
 
Just a warning: Do NOT return a const reference from a getter, except you know that the object you are returning has a longer live span than the function.
  Permalink  
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 3

No, it is not acceptable. It would be if your class member text was declared as a string (not as a pointer).
Consider the following scenario:
#include "HelloWorld.h"
int main()
{
  string foo = "hi";
  HelloWorld hw; // (1) here a new string object is allocated on the heap.

  hw.setText(foo); // (2) here hw.text point to the constant string "hi".

} // (3) here the disaster: the destructor does NOT delete object (1) but tries to delete the constant string of point (2)
  Permalink  
v2
Comments
R. Erasmus at 16-Apr-13 7:37am
   
Ok, it makes sence. Thanks!
CPallini at 16-Apr-13 7:41am
   
You are welcome.
Maciej Los at 16-Apr-13 15:30pm
   
+5

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

  Print Answers RSS
0 OriginalGriff 390
1 Jochen Arndt 165
2 Richard MacCutchan 135
3 DamithSL 95
4 Garth J Lancaster 90
0 OriginalGriff 6,045
1 DamithSL 4,601
2 Maciej Los 4,032
3 Kornfeld Eliyahu Peter 3,480
4 Sergey Alexandrovich Kryukov 3,220


Advertise | Privacy | Mobile
Web01 | 2.8.141220.1 | Last Updated 16 Apr 2013
Copyright © CodeProject, 1999-2014
All Rights Reserved. Terms of Service
Layout: fixed | fluid

CodeProject, 503-250 Ferrand Drive Toronto Ontario, M3C 3G8 Canada +1 416-849-8900 x 100