Inicjowanie statycznego wektora w klasie

0

Panowie (i Panie) wkraczam w świat programowania obiektowo zorientowanego. Jest OK, zagadnienie już nie jest takie abstrakcyjne jak się wydawało na początku. W ramach nauki chciałem pójść o krok dalej i zacząć dynamicznie tworzyć obiekty. No, ale nie obyło się bez przeszkód. Próbuję stworzyć metodę która tworzyłaby nowy obiekt; bez problemu wychodzi mi to używając push_back() w mainie. Zerknijcie proszę na kod i wytknijcie błąd. Nie pogardzę dobrą lekturą na uzupełnienie braków w teorii. Przy okazji możecie powiedzieć czy dobrze użyłem tutaj konstruktora domyślnego. Czy powinienem tyle instrukcji w niego wkładać?

 	#include <iostream>
	#include <string>
	#include <vector>


	class Employee
	{
	private:
		static std::vector<Employee> employees;
		std::string name;
		int age;

		Employee::Employee(std::vector<Employee> employees)
		{
			employees = employees;

			std::string localName;
			int localAge;

			std::cout << "So... do you want to hire a new employee? Let's look at CVs " << std::endl;
			localName = "Marek"; //GenerateName();
			localAge = 21; //these function is not ready yet. it'd be just a rand()
			std::cout << "I've got one. What do u think about " << localName << " age " << localAge << "?" << std::endl;

			int decision;

			do
			{
				std::cout << "Do you want hire him [1] or not [2] ? " << std::endl;
				std::cin >> decision;

				switch (decision)
				{
				case 1:
					name = localName;
					age = localAge;
					decision = 0;
					break;

				case 2:
					//employees.erase(employees.end());
					decision = 0;
					break;

				default:
					std::cout << "Incorrect option. Try again" << std::endl;
				}
			} while (decision != 0);
		}

	public:
		static void InstantiateVector()
		{
			std::vector<Employee> employees;
		}

		static void Employ() //std::vector<Employee> v
		{
			employees.push_back(Employee::Employee(employees));
		}

		static void Display()
		{

		}
	};

	int main()
	{
		std::vector<Employee> EmployeesCollection;

		Employee::Employ();

		system("pause");
	}
0

Mozę powiedz co próbujesz osiągnąć, bo to co tu widzę nie ma najmniejszego sensu.

1
  • Całość zaprojektowana jest bezsensownie - klasa Employee powinna reprezentować pojedynczego pracownika. Dlaczego ona wie cokolwiek o sposobie przechowywania tych pracowników?
  • W konstruktorze dzieje się zdecydowanie zbyt dużo. Konstruktor ma stworzyć i zainicjować obiekt tak, żeby był w stanie prawidłowym. Nic więcej.
  • Całość jest strasznie przekombinowana i nawet nie próbuję tego dokładnie zrozumieć.
0

Przykro mi to słyszeć :( Chcę stworzyć metodę Employ() powołującą do życia nowy obiekt wyołując push_back(). push_back() wywołuje konstruktora w którym dzieje się już cała magia: użytkownik może zatrudnić pracownika o wygenerowanym imieniu i wieku (zaimplementowane, wycięte na poczet uzyskania pomocy). Kod działa pomijając wywołanie metody 'Employee::Employ()' z maina a po prostu używając 'push_back(Employee::Employee(employees))' z poziomu main.

0

Twój statyczny wektor w klasie jest czymś innym niż Twój wektor w mainie.

0

przepraszam post pod postem ale chyba jako niezarejestrowany użytkownik nie mogę edytować własnych postów. Zatem mogę prosić o propozycje? Chetnie poprawie mój kod. Zbiornik obiektów chciałem wepchnąć do klasy aby móc wszystkie obiekty wypisać wywołując metodę. Czy nie tak do tego podchodzę?

0

ups, ten vector w mainie jest nieaktywny. Pozostałość po starym koncepcie. std::vector<Employee> EmployeesCollection; - proszę o tym zapomnieć :D

0

Już pisaliśmy, że podchodzisz do tego bardzo źle.

  • zrób klase reprezentującą pracownika. Ona ma nic nie wiedzieć o tym jak pracowników trzymasz i jak ich tworzysz.
  • zrób funkcję (nie w klasie, po prostu luźną funkcję), która jako argument przyjmuje pracownika i zwraca bool - czy go zatrudnić czy nie.
  • w mainie zbór vector zatrudnionych.
int main () {
  std::vector<Employee> employees;
  Employee some_new_employee;
  if (should_i_employ(some_new_employee))
    employees.push_back(some_new_employee);
} 

Jeśli koniecznie chcesz trzymać ten wektor w Employee (to zły pomysł, ale wyjaśnienie jak to zrobić) trzymaj statyczny wektor, ale w konstruktorze nie przyjmuj jako parametr tego wektora, bo po co?

0

Jeżeli to zły pomysł to oczywiście że nie chcę :) dzięki za radę. Dopytam się dla pewności: nie jest dobrym zwyczajem napisaniem tak kodu aby jak najwięcej rzeczy związanych z klasą była zaimplementowana jako metody? W zamierzeniach chciałem zrobić metody Employee::Employ, Employee::FireEmploy, Employee::DisplayEmployees; a później to rozszerzyć na podklasy.

Wrzucam kod podsunięty przez @pingwindyktator :

 #include <iostream>
#include <string>
#include <vector>

class Employee
{
public:
	std::string name;
	int age;

};

bool should_i_employ(Employee obj)
{
	std::string name;
	int age;
	int decision;

	std::cout << "So... do you want to hire a new employee? Let's look at CVs " << std::endl;
	name = "Marek"; //GenerateName();
	age = 21; //these function is not ready yet. it'd be just a rand()
	std::cout << "I've got one. What do u think about " << name << " age " << age << "?" << std::endl;

	do
	{
		std::cout << "Do you want hire him [1] or not [2] ? " << std::endl;
		std::cin >> decision;

		switch (decision)
		{
		case 1:
			obj.name = name;
			obj.age = age;
			return true;

		case 2:
			return false;

		default:
			std::cout << "Incorrect option. Try again" << std::endl;
		}
	} while (true);
}

int main()
{
	std::vector<Employee> employees;
	Employee some_new_employee;

	if (should_i_employ(some_new_employee))
	{
		employees.push_back(some_new_employee);
	}
	system("pause");
}

Rozumiem że tak powinien wyglądać dobrze napisany kod? Dziękuję za pomoc

0

W should_i_employ dostajesz jako argument tego pracownika, masz wszystkie jego dane. Po co Ci zatem

 std::string name;
    int age;
    int decision;
 
    std::cout << "So... do you want to hire a new employee? Let's look at CVs " << std::endl;
    name = "Marek"; //GenerateName();
    age = 21; //these function is not ready yet. it'd be just a rand() 

Zrób dodatkowo funkcję generate_random_employee, która zwraca pracownika z wygenerowanymi losowo danymi. Wtedy całość powinna wyglądać tak:

int main () {
  std::vector<Employee> employees;
  Employee some_new_employee = generate_random_employee();

  if (should_i_employ(some_new_employee))
    employees.push_back(some_new_employee);
} 
0

ok, rozumiem, masz rację. Już się poprawiam, dzięki.

0

Dlaczego w tym miejscu powinienem to robić? Czy wtedy jeżeli chciałbym zatrudnić więcej pracowników nie musiałbym Employee some_new_employee = Generate() umieścić w pętli? Czy nie powinienem się tego wystrzegać? Czym takie rozwiązanie jest lepsze niż tworzenie "pustego" obiektu na początku a później ewentualne generowanie jego danych w should_i_employee()?

0

should_i_employee jak sama nazwa wskazuje ma Ci powiedzieć tylko to, czy pracownika zatrudnić czy nie. Poczytaj https://en.wikipedia.org/wiki/Single_responsibility_principle

0

teraz nie mam już wątpliwości :) raz jeszcze dzięki

1 użytkowników online, w tym zalogowanych: 0, gości: 1