Adding Range Based Support to Class

[alix]

I am trying to fix, so that range-based for loops are supported with my custom class. I am also trying to search for an employee, but my search gives me wrong results even when strings match?

What's wrong here?

#include <iostream>
#include <string>
#include <iterator>

class Employee
{
public:
  Employee () = default;
  Employee (std::string name, std::string address) {
      this->name = name;
      this->address = address;
  }

  std::string getName ()
  {
    return this->name;
  }
  std::string getAddress ()
  {
    return this->address;
  }

private:
  std::string name = "";
  std::string address = "";

};

class EmployeeArray
{
public:
  EmployeeArray (int max):max (max)
  {
    employees = new Employee[max];
  }

  void add (const Employee * e)
  {
    employees[current_total] = *e;
    current_total++;
  }

  void remove (Employee * e)
  {
    for (Employee * ee = employees; ee != employees + current_total; ++ee)
      {
    	if (ee == e) {
    	    delete ee;
    	    ee = nullptr;
    	    current_total--;
    	    std::cout << "Employee was successfully deleted" << std::endl;
    	}
      }
      
      std::cout << "Employee was not deleted" << std::endl;
  }

  Employee *SearchWithName (std::string name)
  {
    for (Employee * e = employees; e != employees + current_total; ++e)
    {
		if (e->getName() == name) { return e; }
    }
    return NULL;
  }

private:
  Employee * employees;
  int max = 0;
  int current_total = 0;
};

int
main ()
{
  Employee *e =
    new Employee ("Richard Johnson", "1801 E 10th St Pueblo, CO 81001");

  EmployeeArray *empArr = new EmployeeArray (2);
  empArr->add (e);
  empArr->
    add (new Employee ("David Paras", "15 Spring St Rear Peabody, MA 01960"));

  std::cout << (empArr->SearchWithName ("lol") !=
		NULL ? "Employee found!" : "This employee couldn't be found!")
    << std::endl;
  
  for(const Employee &p: *empArr)
  {
      std::cout << p->getName() << std::endl;
  }
  return 0;
}

[coder777]

Your remove (...) function is wrong.

I am also trying to search for an employee, but my search gives me wrong results even when strings match?

What does it mean. In my test the employee is found when using the right name.

You can't have range based for loop with dynamic arrays.

[seeplus]

For a class to work with range-based for loop, the class needs to define:

begin()
end()
operator++() (pre-increment) if needed
operator*() (dereference) if needed
operator->() (pointer reference) if needed
operator !=() if needed

[alix]

@coder777, I am noticing that I cannot perform remove. Why is it wrong?

I am trying to provide: search function, to full match or partial match.

[alix]

begin()
end()
operator++() (pre-increment)
operator*() (dereference)
operator->() (pointer reference)

Where should I put that?

[coder777]

I am noticing that I cannot perform remove. Why is it wrong?

You are trying to delete a field of an array which means undefined behavior. When the order isn't important you can do this:

  void remove (Employee * e)
  {
    for (Employee * ee = employees; ee != employees + current_total; ++ee)
      {
    	if (ee == e) {
    	    current_total--;
    	    *ee = employees[current_total]; // Note: this overwrites the found element with the last
    	    std::cout << "Employee was successfully deleted" << std::endl;
    	    return; // Note: This prevents writing beyond the end
    	}
      }
      
      std::cout << "Employee was not deleted" << std::endl;
  }

It's also a bit strange that you use a pointer for removing.

Actually line 77/82 create memory leaks because on line 39 you just copy the contents of the objects.

to full match or partial match.

For this you may use the find function of the string. See:

http://www.cplusplus.com/reference/string/string/find/

[seeplus]

Consider something like this (which fixes some of the problems in the first post). To support range-based for loop, you need to understand iterators.

#include <iostream>
#include <string>

class Employee
{
public:
	Employee() = default;
	Employee(const std::string& n, const std::string& a) : name(n), address(a) {}

	std::string getName() const { return name; }
	std::string getAddress() const { return address; }

private:
	std::string name;
	std::string address;
};

class EmployeeArray
{
public:
	EmployeeArray() = default;
	EmployeeArray(int m) : max(m), employees(new Employee[m] {}) {}

	~EmployeeArray()
	{
		delete[] employees;
	}

	bool add(const Employee& e)
	{
		if (current_total < max) {
			employees[current_total++] = e;
			return true;
		}

		return false;
	}

	auto begin() const
	{
		return curpos = employees;
	}

	auto end() const
	{
		return employees + current_total;
	}

	auto operator++() const
	{
		++curpos;
		return *this;
	}

	auto operator*() const
	{
		return *curpos;
	}

	bool remove(const Employee& e)
	{
		for (auto ee = begin(); ee != end(); ++ee)
			if (ee->getName() == e.getName()) {
				*ee = employees[--current_total];
				return true;
			}

		return false;
	}

	Employee* SearchWithName(const std::string& name) const
	{
		for (auto e = begin(); e != end(); ++e)
			if (e->getName() == name)
				return e;

		return nullptr;
	}

	void display() const {
		for (auto e = begin(); e != end(); ++e)
			std::cout << e->getName() << std::endl;
	}

private:
	Employee* employees = nullptr;
	int max = 0;
	int current_total = 0;
	mutable Employee* curpos = nullptr;
};

int main()
{
	const auto e = Employee("Richard", "1801 E 10th St Pueblo, CO 81001");
	auto empArr = EmployeeArray(5);

	empArr.add(e);
	empArr.add(Employee("David", "15 Spring St Rear Peabody, MA 01960"));
	empArr.add(Employee("lol", "15 Spring St Rear Peabody, MA 01960"));

	//empArr->display();
	for (const auto& ee : empArr)
		std::cout << ee.getName() << std::endl;

	std::cout << (empArr.SearchWithName("lol") != nullptr ? "Employee found!" : "This employee couldn't be found!") << std::endl;
	std::cout << (empArr.remove(e) ? "Employee deleted!" : "Not deleted") << std::endl;
	std::cout << (empArr.remove(*(empArr.SearchWithName("lol"))) ? "Employee deleted!" : "Not deleted") << std::endl;

	//empArr->display();
	for (const auto& e : empArr)
		std::cout << e.getName() << std::endl;
}

which displays:

Richard
David
lol
Employee found!
Employee deleted!
Employee deleted!
David

[coder777]

@seeplus
The operator++ and operator* are unnecessary and so is curpos.

[seeplus]

@coder777

True in this case as the type returned by begin() is just a pointer - so doesn't need operator overloading. If a non-pointer if returned then they are needed.

#include <iostream>
#include <string>

class Employee
{
public:
	Employee() = default;
	Employee(const std::string& n, const std::string& a) : name(n), address(a) {}

	std::string getName() const { return name; }
	std::string getAddress() const { return address; }

private:
	std::string name;
	std::string address;
};

class EmployeeArray
{
public:
	EmployeeArray() = default;
	EmployeeArray(int m) : max(m), employees(new Employee[m] {}) {}

	~EmployeeArray()
	{
		delete[] employees;
	}

	bool add(const Employee& e)
	{
		if (current_total < max) {
			employees[current_total++] = e;
			return true;
		}

		return false;
	}

	auto begin() const
	{
		return employees;
	}

	auto end() const
	{
		return employees + current_total;
	}

	bool remove(const Employee& e)
	{
		for (auto ee = begin(); ee != end(); ++ee)
			if (ee->getName() == e.getName()) {
				*ee = employees[--current_total];
				return true;
			}

		return false;
	}

	Employee* SearchWithName(const std::string& name) const
	{
		for (auto e = begin(); e != end(); ++e)
			if (e->getName() == name)
				return e;

		return nullptr;
	}

	void display() const {
		for (auto e = begin(); e != end(); ++e)
			std::cout << e->getName() << std::endl;
	}

private:
	Employee* employees = nullptr;
	int max = 0;
	int current_total = 0;
};

int main()
{
	const auto e = Employee("Richard", "1801 E 10th St Pueblo, CO 81001");
	auto empArr = EmployeeArray(5);

	empArr.add(e);
	empArr.add(Employee("David", "15 Spring St Rear Peabody, MA 01960"));
	empArr.add(Employee("lol", "15 Spring St Rear Peabody, MA 01960"));

	//empArr->display();
	for (const auto& ee : empArr)
		std::cout << ee.getName() << std::endl;

	std::cout << (empArr.SearchWithName("lol") != nullptr ? "Employee found!" : "This employee couldn't be found!") << std::endl;
	std::cout << (empArr.remove(e) ? "Employee deleted!" : "Not deleted") << std::endl;
	std::cout << (empArr.remove(*(empArr.SearchWithName("lol"))) ? "Employee deleted!" : "Not deleted") << std::endl;

	//empArr->display();
	for (const auto& e : empArr)
		std::cout << e.getName() << std::endl;
}

[coder777]

If a non-pointer if returned then they are needed.

They are needed in the iterator like objects return by begin()/end() not the container itself.