0

First of all, I have only learned a little bit of Java before. It's been only a few days since I started getting friendly with C++ so please don't take this question so basic and please don't degrade my question.

I made a simple source code as follows:

#include <iostream>

using namespace std;

class Car {
    public:
        void setBrand(string name);
        void setPrice(double price);
        string getBrand();
        double getPrice();
        Car();
        Car(string name);
        Car(string name, double price);

    private:
        string name;
        double price;
};

Car::Car() {
}

Car::Car(string name) {
    name = name;
}

Car::Car(string name, double price) {
    name = name;
    price = price;
}

void Car::setBrand(string name) {
    name = name;
}

void Car::setPrice(double price) {
    price = price;
}

string Car::getBrand(void) {
    return name;
}

double Car::getPrice(void) {
    return price;
}

int main() {
    Car car;
    car.setBrand("Nissan");
    car.setPrice(30000);
    cout << "Brand: " << car.getBrand() << endl;
    cout << "Price: " << car.getPrice() << endl;
    return 0;
}

I wanted to make a code that creates an empty instance of a class called Car, set the field values later and print them out on the console.

The code did not make any errors during the compile, but the result I see was totally different from what I expected. It didn't show the brand name and the price was looking even weird, as follows.

Brand: 
Price: 6.95322e-310

Somebody help me out! Thank you very much indeed in advance.

MarshallLee
  • 1,290
  • 4
  • 23
  • 42
  • 5
    I love the expression *It's been only a few days since I started getting friendly with C++* - Do not worry - in 6 months you will be filling for divorce – Ed Heal Jun 05 '16 at 09:46
  • Tip: Don't give every entity in your program the same name, and you'll be much less likely to confuse yourself. – Kerrek SB Jun 05 '16 at 09:46
  • One idea is to prefix member variables i.e. 'm_name' – Ed Heal Jun 05 '16 at 09:49
  • 3
    The sooner the better, [get rid of that `using namespace std;`](http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-in-c-considered-bad-practice) :) – Quentin Jun 05 '16 at 09:51
  • @Quentin why is it better not to use the namespace? – MarshallLee Jun 05 '16 at 10:06
  • @designspired `using` a namespace breaks it open and pours all of its contents into the containing namespace. The goal of a namespace is to separate a set of names from the outside so they don't clash, `using` one at global scope is sabotaging it. The minimal gain you get when not typing `std::` is offset sooner or later by crazy overloading bugs that depend on the included headers, and that one time you'll update your compiler and need to rename all of your stuff or put back `std::` everywhere because your identifiers clash with new standard names (I've been there). – Quentin Jun 05 '16 at 10:29
  • @Quentin Ahh I see.. that's a really good tip. Thanks mate! – MarshallLee Jun 05 '16 at 10:43

3 Answers3

3

The problem you have is that you override the member names with function parameters. You can use this-> to make it explicit or name the member differently.

For example:

void Car::setBrand(string name) {
    this->name = name;
}

Or:

void Car::setBrand(string new_name) {
    name = new_name;
}
Sorin
  • 11,863
  • 22
  • 26
3

In your constructor and setters, you make no differentiation between the local parameter and the class member.

name = name;

Both the function parameter and the class member are called name. Currently the compiler is assigning the parameter value to itself, and not affecting the class member at all. This is because the function parameter is in a more immediate scope.

Possible solutions:

  1. Specify this when referring to the class member: this->name = name;.
  2. Rename the function parameter: name = _name;.
  3. For the constructor, use initializer lists:

    Car::Car(string name, double price)
    : name(name)
    , price(price)
    { }
    
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
Rotem
  • 21,452
  • 6
  • 62
  • 109
  • @Kerrek Thanks. Any idea why it needed 8 spaces? I thought I had SO markdown figured out by now. – Rotem Jun 05 '16 at 09:57
  • 1
    Because you're in a list, so it's an extra level of hierarchy down. If you want to exit the list and have the code at top level, you can insert something like `

    ` in between (or just some more normal text).
    – Kerrek SB Jun 05 '16 at 09:59
  • @KerrekSB Thanks for that. – Rotem Jun 05 '16 at 10:00
3

There's too much wrong with your code to describe it in prose, so let me present a fixed implementation, and I leave it to you to spot the difference:

#include <string>

class Car
{
private:
    static constexpr double kNoPrice = -1.0;
    static constexpr const char* kNoName = "[no name]";

public:
    // Main constructor: constructs a car with the given name and price.
    Car(std::string name, double price)
    : name_(std::move(name))
    , price_(price)
    {}

    // Convenience constructors:
    Car() : Car(kNoName, kNoPrice) {}
    Car(std::string name) : Car(std::move(name), kNoPrice) {}

    // Accessors:
    const std::string& getBrand() const { return name_; }
    void setBrand(std::string name) { name_ = std::move(name); }

    double getPrice() const { return price_; }
    void setPrice(double price) { price_ = price; }

private:
    std::string name;
    double price;
};

Some random notes, in no particular order:

  • Use correct names. It's std::string, not string, mate or buddy. Never ever be abusing namespace std.
  • Include headers for external names that you need.
  • Reading uninitialized values is undefined behaviour, so none of your constructors should leave fields uninitialized (like price_).
  • Give private members consistent names (e.g. foo_ in my example).
  • Accessors should be const-correct.
  • Convenience constructors should delegate to one single work-horse constructor.
  • Pick sensible defaults for initial values of defaulted fields and make them discoverable.
  • Use move semantics when taking ownership of dynamically managed data (strings, dynamic containers, etc.).
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084