Converting/parsing pointer strings and doubles

Question

Assignment:

Read in info from text file (done)
Retrieve only parts of text file using substr method (done)
Store info into instance variables (need help)

Here is the code I am having trouble with:

string* lati;
lati = new string(data.substr(0, data.find_first_of(",")));

double* latDub;
latDub = new double(atof((char *)lati));

this->latitude = *latDub;

I need to store the latitude into the instance variable latitude.
The variable data is the read-in text file.
this->latitude is declared as a double.

I have tested and the variable lati is the correct value, but once I try to convert it into a double the value changes to 0 for some reason. I am specifically supposed to use the atof method when converting!

Why are you using `new` so much? Why do you use `new` to allocate a `std::string` and a **`double`** of all things? — Nicol Bolas, Sep 18 '11 at 20:25
@Nicol: Likely because he was poisoned by so-called "pure" OO languages that `new` everything. — sbi, Sep 18 '11 at 20:27
Logan, in C++ that is nonsense code. You rarely ever `new` data in C++, and when you do it this should be hidden in some non-dynamic variable. — sbi, Sep 18 '11 at 20:28
Using new is the only way the compiler wouldn't complain! And the header file that we were provided (and cannot change) declared the instance variable of 'latitude' as a double, hence my reason of using a double. — logeyg, Sep 18 '11 at 20:29
@Logan: The compiler will accept automatic objects. If it doesn't, you must have screwed up something else. See my answer. — sbi, Sep 18 '11 at 20:43

score 2 · Accepted Answer · answered Sep 18 '11 at 20:30

2

(char *)lati doesn't do what you think it does. What you're clearly trying to do there is get the char sequence associated with lati, but what you're actually doing is just squeezing a string* into a char* which is all kinds of bad.

There's a member function on std::string that will give you exactly what you want. You should review the documentation for string, and replace (char *)lati with a call to that function.

answered Sep 18 '11 at 20:30

adpalumbo

3,031
12
12

Thank you so much! The member function this scholar and gentleman is referring to is the data() operation, which returns a char*, which is indeed exactly what I wanted. – logeyg Sep 18 '11 at 20:38
@Logan -- actually, you want `c_str()`, not `data()`, but you were close. data() will probably work for you in practice, but is technically incorrect since it isn't guaranteed to be a null-terminated `char` sequence, which is what atof() will be looking for. You should read about both functions, so you can better understand what I mean. – adpalumbo Sep 18 '11 at 20:44

score 0 · Answer 2 · edited May 23 '17 at 10:24

Why your code compiles, but gives meaningless results has already been explained by adpalumbo. There are two fundamental problems in your code leading to that error, on which I want to expand here.

One is that you use a C-style cast: (T)obj. Basically, that just tells the compiler to shut up, you know what you are doing. That is rarely ever a good idea, because when you do know what you are doing, you can usually do without such casts.

The other one is that you are using objects allocated dynamically on the heap. In C++, objects should be created on the stack, unless you have very good reasons for using dynamic objects. And dynamic objects are usually hidden inside objects on the stack. So your code should read like this:

string lati(data.substr(0, data.find_first_of(",")));
double latDub = /* somehow create double from lati */;
this->latitude = latDub;

Of course, latDub is completely unnecessary, you could just as well write to this->latitude directly.

Now, the common way to convert a string into some other type would be streaming it through a string stream. Removing the unnecessary variables you introduced, your code would then look like this:

std::istringstream iss(data.substr(0, data.find_first_of(",")));
if( !iss >> this->latitude ) throw "Dude, you need error handling here!";

Usually you want to pack that conversion from a string into a utility function which you could reuse throughout your code:

inline double convert3double(const std::string& str)
{
  std::istringstream iss(str);
  double result;
  if( !iss >> result )
    throw std::exception("Dang!");
  return result;
}

However, since the very same algorithm can be used for all types (for which operator>> is overloaded meaningfully with an input stream as the left operand), just make this a template:

template< typename T >
inline T convert3double(const std::string& str)
{
  std::istringstream iss(str);
  T result;               // presumes default constructor
  if( !iss >> result )    // presumes operator>>
    throw std::exception("Dang!");
  return result;
}

Wow. Thanks a ton.. very useful information. I 'just' started a c++ class at school, so I think the prof would be very surprised if I came in with a solution like this, haha. Thanks again though, this has added a lot to my new understanding of C++. — logeyg, Sep 18 '11 at 20:41
@Logan: You are welcome. I might add, though: You need a [good C++ book](http://stackoverflow.com/q/388242/140719) to really learn C++. — sbi, Sep 18 '11 at 20:45

Converting/parsing pointer strings and doubles

2 Answers2