Problem with Accessing Variables

Hello! Thank you for taking the time to help me out. I am having a problem with a program that I wrote, which is supposed to keep information about a CPU and a Computer, such as brand name, memory, and other details. The problem I am having is with the setCPU function inside the Computer.cpp file, almost at the bottom of the code I am posting. This function is supposed to set the details about the CPU, but the problem I am having is with accessing the variables of the CPU.h file inside the Computer.cpp file. I have done "#include CPU.h" but it doesn't work. Can someone help me figure out how to access the variables type and speed in Computer.cpp?

Thank you in advance for any help you can provide! Here are three of my files, CPU.h, Computer.h, and Computer.cpp (I think they are the relevant ones):

Code:

// CPU.h

// protections 
#ifndef CPU_H
#define CPU_H

using namespace std;

class CPU
{
      // private variables
      private:
        string type;
        int speed;
        
      // public functions
      public: 
        CPU();
        ~CPU();
        string getType();
        int getSpeed();
        void setType(string type);
        void setSpeed(int speed);
        void printInfo();
};

#endif

Code:

// Computer.h

// protectors
#ifndef Computer_H
#define Computer_H

using namespace std;

// CPU class so that everything compiles correctly
class CPU;

class Computer
{
      // private variables
      private:
        string brandName;
        CPU *cpu;
        int memory;
        double price;
        
      // public functions  
      public: 
        Computer();
        ~Computer();
        string getBrandName();
        CPU * getCPU();
        int getMemory();
        double getPrice();
        void setBrandName(string newBrandName);
        void setCPU (string cpuType, int cpuSpeed);
        void setMemory (int memoryAmount);
        void setPrice (double newPrice);
        void printInfo();
};

#endif

Code:

// Computer.cpp

#include <iostream>
#include <string>
#include <cstdlib>
#include "Computer.h"
#include "CPU.h"
using namespace std;

// constructor
Computer::Computer()
{
    brandName = "?";
    CPU *cpu = new CPU();
    memory = 0;
    price = 0.0;
}
        
// destructor
Computer::~Computer()
{
    delete cpu;
    cout << "The computer " << brandName << " is being destroyed." << endl;
}


// mutator functions set the variables to the new values
void Computer::setCPU (string cpuType, int cpuSpeed)   
{    
     *cpu->type = cpuType;
     *cpu->speed = cpuSpeed;     
}

Code:

Computer::Computer()
{
    brandName = "?";
    CPU *cpu = new CPU();
    memory = 0;
    price = 0.0;
}

This declares a local CPU* which happens to shadow the name of the class member.

Originally Posted by vileoxidation

[...]

Code:

// CPU.h

// protections 
#ifndef CPU_H
#define CPU_H

using namespace std;

class CPU
{
      // private variables
      private:
        string type;
        int speed;
        
      // public functions
      public: 
        CPU();
        ~CPU();
        string getType();
        int getSpeed();
        void setType(string type);
        void setSpeed(int speed);
        void printInfo();
};
#endif

[...]

Code:

// Computer.cpp
[...]
void Computer::setCPU (string cpuType, int cpuSpeed)   
{    
     *cpu->type = cpuType;
     *cpu->speed = cpuSpeed;     
}

you made "type" and "speed" private members of class "CPU", but your trying to access it outside of that class, in "Computer". private variables are only visible in the class they are declared.

also, you made getters and setters for the "type" and "speed" variables, which is good. however, your not using them! the whole point of a getter is to allow another class, who cannot "see" a private variable, to modify it. if you have a private member of a class that you think someone might want to be able to control (type, speed) then you create getters and setters for them, so whoever wants to read/write these variables they do so through the get<variable>() and set<variable>() functions.

Originally Posted by anon

Code:

Computer::Computer()
{
    brandName = "?";
    CPU *cpu = new CPU();
    memory = 0;
    price = 0.0;
}

This declares a local CPU* which happens to shadow the name of the class member.

Okay...how do I declare the cpu object other than this way? The assignment description says this:

The following constructor should be provided to initialize each attribute:
Computer()
This constructor initializes all strings to "?", all integers to 0, and all double to 0.0 and allocates memory for the pointer of of CPU (i.e., call the constructor of the CPU class).

So I don't understand how else to do this except for in this way. Can you help me figure out how to instantiate the cpu object?

Thank you for your help!!

Originally Posted by vileoxidation

Code:

// Computer.h
[...]
class Computer
{
      // private variables
      private:
        string brandName;
        CPU *cpu;
        int memory;
        double price;
[...]
};

#endif

Code:

// Computer.cpp
[...]
Computer::Computer()
{
    brandName = "?";
    CPU *cpu = new CPU();
    memory = 0;
    price = 0.0;
}
[...]

you already declared the variable "cpu" in the header of "Computer". to access it in "Computer.cpp" you simply use the variable name, the same way you with memory, price, etc.

however, your problem is a combination of this and my other comment above.

Originally Posted by nadroj

you made "type" and "speed" private members of class "CPU", but your trying to access it outside of that class, in "Computer". private variables are only visible in the class they are declared.

also, you made getters and setters for the "type" and "speed" variables, which is good. however, your not using them! the whole point of a getter is to allow another class, who cannot "see" a private variable, to modify it. if you have a private member of a class that you think someone might want to be able to control (type, speed) then you create getters and setters for them, so whoever wants to read/write these variables they do so through the get<variable>() and set<variable>() functions.

Okay, yeah, I thought that might be the problem. My issue is that this is for an assignment, and in the assignment description, my teacher said that those variables are supposed to be private. So I don't know what to do about that?

Okay, I think I use the setter, which makes sense. I don't know how to use the set<variable>() function though...do I do this:

Code:

void Computer::setCPU (string cpuType, int cpuSpeed)   
{   
     *cpu->setType(cpuType);
     *cpu->setSpeed(cpuSpeed);     
}

? I'm not thinking that is right, but I'm trying!

Thank you for the help!!

exactly. instead of "cpu->type = 5" you use the setter and do "cpu->setType(5)";. to read the value of this private variable, you use the "getType()" function.

EDIT: the main thing behind a setter is you can control the input. lets say for your CPU type you only expect the values 1, 2, and 3 (or whatever it may be). using your current setter, you can do "cpu->setType(-10000)" and the CPU class will blindly accept this. later on when you go to read this value (getType()) you would only expect valid values (1, 2, 3), however you would get back -10000 in which case that might cause problems because your code that depends on this didnt anticipate this value.

you could modify your setter do validate it, ie:

Code:

void setType(int input)
{
   if ( input > 0 && input < 4)
   {
       type = input; // accept it
   }
   else
   {
      type = 1; // some "default" VALID value
   }

this will ensure that only valid values are saved. you could modify it further to tell the caller whether its value was valid or not, i.e.:

Code:

int setType(int input)
{
   if ( input > 0 && input < 4)
   {
       type = input; // accept it
       return 1; // "true" ie valid input
   }
   else
   {
      return 0; // "false", invalid entry
   }

the caller should then check the return value and if it was false/0 it means that the user's input was invalid, so it should loop over the process of getting input from user (or whatever), trying to set it, if it was true then continue otherwise ask for it again, etc.

this is just a heads up, because you probably dont need to validate it in your assignment, but be aware of why a setter exists.

Okay, cool. So I think I did that part right. Now, I am having another problem, though. When I run the program, and enter the information, after I enter the CPU type it prints the next prompt, doesn't wait for user input, and just asks the user what action they would like to perform. What is going wrong here?

Here is the Assignment6.cpp file, as well as the updated Computer.cpp file:

Code:

// Assignment #: 6

#include <iostream>
#include <string>
#include <cstdlib>
#include "CPU.h"
#include "Computer.h"

using namespace std;

void printMenu();

int main()
{
	// local variables, can be accessed anywhere from the main method
	char input1 = 'Z';
	string inputInfo;
	string brandName;
	double price;
	int memory;
	string cpuType;
	int cpuSpeed;
	string line;

	// allocate memory for the pointer of the Computer
	Computer * computer1 = new Computer();

	printMenu();

	do  // will ask for user input
	{
		cout << "What action would you like to perform?\n";
		input1 = getchar();
		input1 = toupper(input1);

		// matches one of the case statement
		switch (input1)
		{
		case 'A':   //Add Computer
			cout << "Please enter the computer information:\n";
			cout << "Enter a brand name:\n";
			cin >> brandName;
			computer1->setBrandName(brandName);

			cout << "Enter a computer price:\n";
			cin >> price;
			computer1->setPrice(price);

			cout << "Enter a computer memory:\n";
			cin >> memory;
			computer1->setMemory(memory);

			cout << "Enter a cpu type:\n";
			cin >> cpuType;
			cout << "Enter a cpu speed:\n";
			cin >> cpuSpeed;
			computer1->setCPU(cpuType, cpuSpeed);
			break;
		case 'D':   //Display computer
			computer1->printInfo();
			break;
		case 'Q':   //Quit
			delete computer1;
			break;
		case '?':   //Display Menu
			printMenu();
			break;
		default:
			cout << "Unknown action\n";
			break;
		}

		getchar(); //to flush '\n' 
	} while (input1 != 'Q');

	return 0;
}


/** The method printMenu displays the menu to a user**/
void printMenu()
{
	cout << "Choice\t\tAction\n";  
	cout << "------\t\t------\n";  
	cout << "A\t\tAdd Computer\n";  
	cout << "D\t\tDisplay Computer\n";
	cout << "Q\t\tQuit\n"; 
	cout << "?\t\tDisplay Help\n\n";
}

Code:

// Computer.cpp

#include <iostream>
#include <string>
#include <cstdlib>
#include "Computer.h"
#include "CPU.h"
using namespace std;

// constructor
Computer::Computer()
{
    brandName = "?";
    cpu = new CPU();
    memory = 0;
    price = 0.0;
}
        
// destructor
Computer::~Computer()
{
    delete cpu;
    cout << "The computer " << brandName << " is being destroyed." << endl;
}


// accessor functions return the variables 
string Computer::getBrandName()
{   return brandName;        }
CPU * Computer::getCPU()
{   return cpu;       }
int Computer::getMemory()
{   return memory;    }                         
double Computer::getPrice()
{   return price;     }


// mutator functions set the variables to the new values
void Computer::setBrandName(string newBrandName)
{    brandName = newBrandName;     }
void Computer::setCPU (string cpuType, int cpuSpeed)   
{   
     cpu->setType(cpuType);
     cpu->setSpeed(cpuSpeed);     
}
void Computer::setMemory (int memoryAmount)
{    memory = memoryAmount;         }
void Computer::setPrice (double newPrice)
{    price = newPrice;              }


// printInfo function prints the information about the computer in the 
// desired format
void Computer::printInfo()
{    cout << "\nBrandName:\t" << brandName << "\nCPU:\t\t" << cpu->getType() << "," << cpu->getSpeed() << "HZ\nMemory:\t\t" << memory << "M\nPrice:\t\t$" << price << "\n\n" << endl;    }

I'm sure there's a valid use for getchar() somewhere. This isn't it. Use << and be done with it.

Okay. I think I actually got it working! It turns out that I was entering the input incorrectly, because I was putting spaces in where they didn't belong. So I think it is working! I submitted my files to the online compiler that we use, and the output from the program looked correct.

Thank you for all of the help on this!!

Stylistically, you want to avoid putting a "using namespace std;" directive in a header file and instead explicitly qualify any necessary object with std:: in said header. Also, the way you've got things written currently, you would have problems if anyone happened to reorder the header file inclusions in your source files (putting computer.h and cpu.h before <string> for example). You want to avoid forcing people who use your stuff to remember to put such things in a specific order. For that you'd need to include the necessary header file(s) (<string> in this case) within the computer.h and cpu.h headers.