1

I am creating a rental system with 3 classes "Vehicle","car" & "Rentalsystem".Now when it asks me which car do I input the car name lets say for instance "Alto", it replies that "Sorry this car isn't available" . Why?

#include<iostream>
#include<fstream>
using namespace std;

class Vehicle{
    public:
        string name;
        int seats;
        int model,totalrent;
};

class Car:public Vehicle{
    public:
        string AvailableCars[4] = { "Alto", "Mercedes", "Corolla", "Audi" };
        int CarStock[4] = { 4, 4, 4, 4 };  // Stock for each car type
    
        void ShowDetails(){
            cout << "Available Cars are:";
            for(int i=0;i<4;i++){
                cout << AvailableCars[i] << endl;
            }
            cout << "Enter the car name you would like to rent";
            cin >> name;
        }
        
        void CheckInput(){
            for(int i=0;i<4;i++){
                if(AvailableCars[i]==name){
                    cout << "Calculating rent!";
                    if(name==AvailableCars[0]){
                        seats=5;
                        totalrent=seats*10000;
                        cout << "Total Rent:" << totalrent;
                        break;
                    }
                    else if(name==AvailableCars[1]){
                        seats=2;
                        totalrent=seats*10000;
                        cout << "Total Rent:" << totalrent;
                        break;
                    }
                    else if(name==AvailableCars[2]){
                        seats=3;
                        totalrent=seats*10000;
                        cout << "Total Rent:" << totalrent;
                        break;
                    }
                    else {
                        seats=4;
                        totalrent=seats*10000;
                        cout << "Total Rent:" << totalrent;
                        break;
                    }
                    break;
                }
                else{
                    cout << "Sorry this car isn't available!";
                    break;
                }
            }
        }
};

class RentalSystem:public Car{
    public:
        void CalculateRent(){
            CheckInput();
        }
};

int main(){
    int choice;
    cout << "Enter choice pls";
    cin >> choice;
    if(choice == 1){
        Car c1;
        c1.ShowDetails;
RentalSystem r1;
        r1.CalculateRent();
    }
    else if(choice != 1 && choice != 2){
        cout << "Sorry choose between available options.";
    }
}

I have tried: using breaks. using bool.

expected: "Calculaating rent....." got:"Sorry this car..."

tadman
  • 208,517
  • 23
  • 234
  • 262
  • 3
    Instead of two unrelated arrays, consider an array of name/stock pairs, or a simple `struct`. – tadman Aug 25 '23 at 19:09
  • You *really* need to learn how to use a simple `for` loop instead of this hard-coded check here. That `if` chain also hints that you need to include `seats` and `rent` properties in this car `struct`. – tadman Aug 25 '23 at 19:12
  • Turn on compiler warnings. Fix compiler warning. ???. Profit. – Eljay Aug 25 '23 at 19:13
  • I get a compiler error: https://godbolt.org/z/TTGe6zW7f – user4581301 Aug 25 '23 at 19:20
  • `c1.ShowDetails;` -> `c1.ShowDetails();` – Remy Lebeau Aug 25 '23 at 20:13
  • Why does class `Car` have a container of manufacturer names? Does it make sense for a Car to have Available Cars? Hmmm, perhaps a banana should have a container of available bananas. – Thomas Matthews Aug 25 '23 at 20:39
  • What source are you using to learn from? Where does it teach that a single item (Car) should have multiple instances of the same item? Have you heard of containers like `std::vector`, `std::list` or `std::map` that hold multiple instances of a class? – Thomas Matthews Aug 25 '23 at 20:42
  • I was expecting `class Car` to be a specialization of a `Vehicle`. A `Boat` can be a specialization of `Vehicle`, a bicycle or motorcycle **is-a** `Vehicle`. In your case, the class `Car` (signular) seems to be a collection of vehicles (plural). If `Car` was singular, you could have `std::vector Parking_Lot;`, which makes more sense. – Thomas Matthews Aug 25 '23 at 20:46
  • Also, a `RentalSystem` should not inherit from `Car`, because you can't drive a `RentalSystem` and can't park the `RentalSystem` in a driveway. `RentalSystem` should **contain** zero or more `Car`, thus having a `std::vector`. Reminder, usually inheritance satisfies the **is-a** relationship and the child class should be able to be substituted for the parent in functions that take parent instances. – Thomas Matthews Aug 25 '23 at 20:49

3 Answers3

3

The reason your code fails is because you are reading the user's input into c1.name (actually, you are not, because you are not actually calling c1.ShowDetails since you are missing the () call operator on it) and then you are processing r1.name instead, which has no value assigned to it. c1.name and r1.name are two completely different and unrelated variables in memory. Just because Car and RentalSystem both inherit the name member from Vehicle does not make c1.name and r1.name the same variable.

That said, just about everything in your code is plain wrong. Most of the Vehicle members are unused. RentalSystem should not derive from Car at all. And AvailableCars and CarStock belong in RentalSystem instead of Car.

I would suggest something more like the following instead:

#include <iostream>
using namespace std;

class Vehicle {
    public:
        string name;
        int seats;
};

class Car : public Vehicle {
};

class RentalSystem {
    public:
        Car AvailableCars[4] = {
            {"Alto", 5},
            {"Mercedes", 2},
            {"Corolla", 3},
            {"Audi", 4}
        };
        int CarStock[4] = { 4, 4, 4, 4 };  // Stock for each car type
        std::string carName;

        void ShowDetails() {
            cout << "Available Cars are:";
            for(int i = 0; i < 4; ++i){
                cout << AvailableCars[i].name << endl;
            }
            cout << "Enter the car name you would like to rent: ";
            cin >> carName;
        }

        void CalculateRent() {
            for(int i = 0; i < 4; ++i){
                if ((AvailableCars[i].name == carName) && (CarStock[i] > 0)) {
                    cout << "Total Rent:" << (AvailableCars[i].seats * 10000) << endl;
                    return;
                }
            }
            cout << "Sorry this car isn't available!" << endl;
        }
};

int main(){
    int choice;
    cout << "Enter choice pls";
    cin >> choice;
    if (choice == 1) {
        RentalSystem r1;
        r1.ShowDetails();
        r1.CalculateRent();
    }
    else {
        cout << "Sorry choose between available options.";
    }
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
1

The reason why your code isn't working is because you are trying to call the CalculateRent() method from a different object than the one that is holding the actual name that you retrieved during the c1.ShowDetails(); method.

While the RentalSystem does indeed inherit the same variables of the Car class, this doesn't mean that those variables are available to it after being instantiated, meaning that each separate instance of Car and RentalSystem have their own unique variable called name.

To solve this, you could write a method that takes in (a specific instance of) your Car class, and assigns that object's member name to the RentalSystem class, as follows:

class RentalSystem:public Car{
    public:
        void CalculateRent(Car & Car){
            name = Car.name;
            CheckInput();
        }
};

int main()
{
    int choice;
    std::cout << "Enter choice pls\n";
    std::cin >> choice;
    if(choice == 1){
        Car c1;
        c1.ShowDetails();
        RentalSystem r1;
        r1.CalculateRent(c1);
    }
    else if(choice != 1 && choice != 2){
        std::cout << "Sorry choose between available options.";
    }
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Valdez
  • 46
  • 3
  • `RentalSystem` should not derive from `Car`, that just makes no sense. A system acts on vehicles, it is not itself a vehicle – Remy Lebeau Aug 25 '23 at 20:14
  • thank you so much and please answer this question, r1.CalculateRent(c1); In this part are we passing data of object c1 to Rental class function? – Zaynab Azam Aug 25 '23 at 20:24
  • @RemyLebeau in my final, my instructor asked us to do so . Why do you think it does not make sense as Rentalsystem will further check the data derived from Car class and look for stock. – Zaynab Azam Aug 25 '23 at 20:27
  • 2
    @ZaynabAzam then your instructor is not teaching you C++ properly. What you have is full of bad design practices. – Remy Lebeau Aug 25 '23 at 20:52
  • I think the idea the instructor is conveying is inheritance. There are ways for you to inherit member objects that are universal using the static keyword, because maybe a rental system has to have the same inventory list that you can expect the object "car" to be if someone were to rent it. It is a super hypothetical example and its mostly meant to demonstrrate the concept of inheritance... don't get too hung up on the details. – Valdez Aug 27 '23 at 01:32
1

The errors in your code were this:

  • c1.ShowDetails; should be c1.ShowDetails();
  • Car c1 and RentalSystem r1 are different objects and you ask the user for a car name in c1 and try to use this name in r1.

When you fix them both, your program should run in a way you might expect.


However, there are still a few design flaws (some of which are already mentioned in the comments under your question) in your code. The two most common ones are this "using namespace std" and "magic numbers".

using namespace std;

First of all, don't do using namespace .... This will just put everything from the namespace std into the global namespace and might cause some naming collisions. In some cases (I use it for e.g. std::chrono_literals) using this makes your code easier to read and is not that bad, but only in the innermost scope possible and not just globally. You can check out Why is "using namespace std;" considered bad practice? for reasons why not to use it.

Magic numbers

In your main() function, you write if(choice == 1). I read your code and have no idea what 1 stands for, not even a guess. Therefore it will be hard for someone else (and probably you in a few weeks/months) to read/edit/maintain your code. I would recommend doing something like this:

constexpr int option1{ 1 };    // but give it a meaningful name, not like I just did
if (choice == 1) {
    // ...
}

But you should of course use a meaningful name to it, since option1 doesn't tell me what it is either. You could check What is a magic number, and why is it bad? out aswell.

By the way I would recommend to use a better name for choice as well, since this would help me to understand what option1 is even better. So in general don't use magic numbers and give your variables (and functions, classes, whatever) meaningful names.

DRY principle (Don't repeat yourself)

If I look at your for loop in Car::CheckInput() and other loops, there is much repetition going on. This makes your code way harder to change, since you have to change it in all those places. You use

seats = 5;
totalrent = seats * 10000;
cout << "Total Rent:" << totalrent;
break;

4 times, which just small differences (only the amount of seats changes). And pay attention to that 4 in your loops. You have it in each loop. If you get an additional car or get rid of one, you need to change this 4 in each single place.

Objects

As the comments under your question already say, you have string AvailableCars[4], int CarStock[4] and a "standalone" amount of seats floating around.

Now imagine your dealership gets a new car. You have to read your whole code and try to find each place where you need to add information about this car. It would be much easier to have all information bundled in one place. You should try to restructure your classes to avoid this.

Further RentalSystem is derived from Car. Why is that? Is your RentalSystem some sort of car? Maybe a cabrio? RentalSystem should have a member which stores Cars. Your "free floating" arrays from Car should be in there. This would also make the Car class redundant, but you might add other Vehicles and additional functionality to your Car later on so I think this is fine.

Finding the chosen car

Your for loop in Car::CheckInput() contains something like if (name == AvailableCars[0]) in it. With the index going from 0 up to 2 (which is the index before the last plus for the last a else). This defies the whole reason why you would use a array and a loop and you could not just add a new Car to your "free floating" arrays, you would also need to adjust this, which is just unnecessary work.


There are still some minor things which I didn't mention yet.

  • I notices you #include <fstream> but don't use it and you didn't #include <string> (which could already be in #include <iostream>, but I would recommend still including it). (and please put a blank between the "#include and <iostream> even it is the same for the compiler :D)
  • When you ask the user for input, tell them what exactly you expect from them (what does "Enter choice pls" mean exactly?) and put there some additional space too, like "Enter choice pls: ", so it doesn't look like "Enter choice pls5" after they wrote something.
  • Always initialize your variables

My solution

#include <iostream>     // please add space between "#include" and the file :D just a personal preference though
#include <string>       // you use the class "std::string", so include it even if it might already get included with "iostream"
#include <array>        // std::array has many advantages over c-style arrays, so I prefer it, use std::vector if you need to resize your array
#include <algorithm>    // for std::find_if()

class Vehicle {
public:
    Vehicle(const std::string& name, int seats)
        : name(name), seats(seats) {
    }

    std::string name{};
    int seats{};
    //int model, totalrent;     // you never use model, so why does it exists? If you want to add something later, its fine
    // declaring multiple variables on the same line can lead to errors when using raw pointers (which you shouldnt anyways)
    // so you might just do it on multiple lines
    //int totalrent{};  // this should not be here, since the dealer calculates a rent, not the vehicle itself
};

// if you only ever have "Car" and no other "Vehicle" and you don't think of extending either one you don't need to have a sperate class
class Car : public Vehicle {
public:
    Car(const std::string& name, int seats)
        : Vehicle(name, seats){
    }

    void ShowDetails() const {
        std::cout << "Name: " << name << ", ";
        //std::cout << "Model: " << model << ", ";     // if you decide to use "model", uncomment this
        std::cout << "Seats: " << seats << std::endl;
    }
};

// this should not inherit Car, since it is not a car, but might include multiple cars
class RentalSystem {
public:
    void SimulateInteraction() {
        ShowDetalis();

        std::string carName{ GetInput() };
        auto inputCarIter{ std::find_if(stock.begin(), stock.end(), [&carName](const StockItem& item) {
            return item.car.name == carName;
            }) };

        if (inputCarIter == stock.end()) {
            std::cout << "Sorry this car isn't available!" << std::endl;
            return;
        }

        constexpr int basePrice{ 10'000 };  // I gues this is what your 10'000 is supposed to be?
        int rent{ inputCarIter->car.seats * basePrice };
        std::cout << "Total Rent: " << rent << std::endl;
        inputCarIter->numberOfAvailableCars--;  // if you rent one, decrement the amount of cars you have
    }

// I have seperated this classes members and methods into public and private, since a user might not
// need or want to know everything about the details of your rental
private:
    // don't have to seperate arrays which contain data which is related to each other
    // you could use a std::pair<> for this too
    struct StockItem {
        Car car;
        int numberOfAvailableCars;
    };

    std::array<StockItem, 4> stock{
        StockItem(Car("Alto",       5), 4),
        StockItem(Car("Mercedes",   2), 4),
        StockItem(Car("Corolla",    3), 4),
        StockItem(Car("Audi",       4), 4)
    };
    
    void ShowDetalis() const {
        std::cout << "Available Cars are:" << std::endl;    // use an additional std::endl
        for (const auto& item : stock) {
            // only print them if they are still available
            if (item.numberOfAvailableCars > 0) {
                item.car.ShowDetails();
            }
        }
    }

    std::string GetInput() {
        std::cout << "Enter the car name you would like to rent: ";

        std::string carName{};
        std::cin >> carName;
        return carName;
    }
};

int main() {
    int choice{};

    std::cout << "Enter choice pls: ";
    std::cin >> choice;

    if (choice == 1) {
        RentalSystem rental;
        rental.SimulateInteraction();
    }
    // else if (choice != 1 && choice != 2)     // (choice != 1) will always be true
    // there is no additional branch for when the user inputs 2 since I dont know what it means
    else {
        std::cout << "Sorry choose between available options." << std::endl;
    }

    return 0;
}

If you have questions on something like lambda expressions (with the weird [](){} syntax), std::find_if, std::array or something else feel free to leave a comment and I will try to explain them a bit.

I can recommend Learn C++ and The Definitive C++ Book Guide and List for learning all the basics and cppreference if you ever need to look something up.

Joel
  • 721
  • 1
  • 13