Comparer question with custom Objects

[dbag]

I have the following list of person class to which I am sorting by full name and I apologize if this is a 101 question as I am still new to C++ and come from a C# background.

My question is why does the comparer work when its defined inline to the object being used but throw a compile error when you try and pass in a comparer that resides inside a class.

Person.h

#include <string>

namespace Test
{
	class PersonExample
	{
	public:
		PersonExample();
		PersonExample(std::string LastName, std::string MiddleName, std::string FirstName);
		~PersonExample();
		std::string getFirstName();
		std::string getMiddleName();
		std::string getLastName();
		std::string getFullName();
		void setFirstName(std::string str);
		void setMiddleName(std::string str);
		void setLastName(std::string str);
		bool compare_lastName(PersonExample& first, PersonExample& second);
	private:
		std::string firstName;
		std::string middleName;
		std::string lastName;
	};
}#include <string>

namespace Test
{
	class PersonExample
	{
	public:
		PersonExample();
		PersonExample(std::string LastName, std::string MiddleName, std::string FirstName);
		~PersonExample();
		std::string getFirstName();
		std::string getMiddleName();
		std::string getLastName();
		std::string getFullName();
		void setFirstName(std::string str);
		void setMiddleName(std::string str);
		void setLastName(std::string str);
		bool compare_lastName(PersonExample& first, PersonExample& second);
	private:
		std::string firstName;
		std::string middleName;
		std::string lastName;
	};
}

Person.Cpp

namespace Test
{
	PersonExample::PersonExample()
	{
		this->firstName = "";
		this->middleName = "";
		this->lastName = "";
	}
	
	PersonExample::PersonExample(std::string LastName, std::string MiddleName, std::string FirstName)
	{
		this->firstName = FirstName;
		this->middleName = MiddleName;
		this->lastName = LastName;
	}

	PersonExample::~PersonExample()
	{
	}

	std::string PersonExample::getFirstName()
	{
		return this->firstName;
	}

	std::string PersonExample::getMiddleName()
	{
		return this->middleName;
	}

	std::string PersonExample::getLastName()
	{
		return this->lastName;
	}

	std::string PersonExample::getFullName()
	{
		return this->lastName + ", " + this->firstName + " " + this->middleName;
	}

	void PersonExample::setFirstName(std::string str)
	{
		this->firstName = str;
	}

	void PersonExample::setMiddleName(std::string str)
	{
		this->middleName = str;
	}

	void PersonExample::setLastName(std::string str)
	{
		this->lastName = str;
	}

	bool PersonExample::compare_lastName(PersonExample& first, PersonExample& second)
	{
		//Local Variable Declaration
		unsigned int i = 0;
		
		while ((i<first.getLastName().length()) && (i<second.getLastName().length()))
		{
			if (tolower(first.getLastName()[i])<tolower(second.getLastName()[i])) return true;
			else if (tolower(first.getLastName()[i])>tolower(second.getLastName()[i])) return false;
			++i;
		}
		return (first.getLastName().length() < second.getLastName().length());
	}
}

Entry Point:

bool compare_fullName(Test::PersonExample& first, Test::PersonExample& second)
{
	//Local Variable Declaration
	unsigned int i = 0;

	while ((i<first.getFullName().length()) && (i<second.getFullName().length()))
	{
		if (tolower(first.getFullName()[i])<tolower(second.getFullName()[i])) return true;
		else if (tolower(first.getFullName()[i])>tolower(second.getFullName()[i])) return false;
		++i;
	}
	return (first.getFullName().length() < second.getFullName().length());
}

int _tmain(int argc, _TCHAR* argv[])
{
Test::PersonExample p;
	std::list<Test::PersonExample> people = { Test::PersonExample("Corcoran", "Aaron", "Joshua"), Test::PersonExample("Corcoran", "Marie", "Corinne") };
	std::list<Test::PersonExample>::iterator peopleIT;
	people.sort(compare_fullName);
        //people.sort(p.compare_fullName); //Compile Error
 
	for (peopleIT = people.begin(); peopleIT != people.end(); ++peopleIT)
	{
		std::cout << ' ' << peopleIT->getFullName() << endl;
	}
}

[TheIdeasMan]

Hi,

//people.sort(p.compare_fullName); //Compile Error

The PersonExample class does not have a compare_fullName function.

Some other things:

There is no need to use this-> like you do, a class has direct access to it's members. Only use it if you need to disambiguate.

Pass std::string or any container by const reference, and use a member initialization list in the constructor:

PersonExample::PersonExample(const std::string& LastName, 
                                     const std::string& MiddleName , 
                                     const std::string& FirstName)
         : // colon introduces member init list
        
		firstName (FirstName),  // direct initialization, use brace initialization for numbers to prevent narrowing
		middleName (MiddleName),
		lastName (LastName)
	{
       // if required do validation here
        } 

http://en.cppreference.com/w/cpp/language/initializer_list
http://en.cppreference.com/w/cpp/language/initialization

I like to name my constructor parameters the same as the members with Arg appended.

Rather than use iterators, consider using a range based for loop when all the items of the container are are accessed.

http://en.cppreference.com/w/cpp/language/range-for

for (auto&& item : people // auto&& is a forwarding reference with auto type deduction
	{
		std::cout << ' ' << item.getFullName() << "/n";
	}

Any particular reason to use std::list ? I don't see anything to preclude using a std::vector.

Good Luck !!

[dbag]

Hey @TheIdeasMan,

Thank you for the explanation and reference links. The reason I am using the list over the vector is just to basically get a better understanding of them and how I can use them. I come from C# and we don't have a vector concept and all generics are based really off lists. I apologize I forgot to include the code I was using for the PersonExample comparer, I have included it below.

passing the const version simply prevents manipulation of the original variable is that correct?

bool PersonExample::compare_lastName(PersonExample& first, PersonExample& second)
	{
		//Local Variable Declaration
		unsigned int i = 0;
		
		while ((i<first.getLastName().length()) && (i<second.getLastName().length()))
		{
			if (tolower(first.getLastName()[i])<tolower(second.getLastName()[i])) return true;
			else if (tolower(first.getLastName()[i])>tolower(second.getLastName()[i])) return false;
			++i;
		}
		return (first.getLastName().length() < second.getLastName().length());
	}

[TheIdeasMan]

I come from C# and we don't have a vector concept and all generics are based really off lists.

In C++ std::vector is a really good container, I have been surprised a number of times where experts here have explained & shown how efficient it is.

passing the const version simply prevents manipulation of the original variable is that correct?

Yes, and applying it to a member functions means it can't alter the state of the class object. That is, it can't alter the value of any of the member variables. This is called const correctness and is a very good thing. So you could do this with your compare functions for example:

bool PersonExample::compare_lastName(PersonExample& first, PersonExample& second) const
{

}

std::string has it's own operator< so you shouldn't have to write your own comparison function, just sort the list.


In case you do want to compare strings there is a lexicographical_compare algorithm:

http://en.cppreference.com/w/cpp/algorithm/lexicographical_compare

It would reduce your code a little.


Maybe you function doesn't satisfy the compare concept?

http://en.cppreference.com/w/cpp/concept/Compare

[dbag]

@TheIdeasMan,

So if I simply called compare without specifying the custom comparer method how will it know which of the getter methods I wish to do the compare on? If this time I wanted to compare and sort via lastname but next time I want to sort on Firstname, etc?

Originally I was using the const in my comparer method but when I attempted to do first.getLastName().length() it would tell me length() did not exist. So, in order to get back the length method I removed the const.

Does that help explain some things?

[TheIdeasMan]

So if I simply called compare without specifying the custom comparer method how will it know which of the getter methods I wish to do the compare on? If this time I wanted to compare and sort via lastname but next time I want to sort on Firstname, etc?

You are right, I realized that after I posted my reply. I applied the strikeout to that part.

Originally I was using the const in my comparer method but when I attempted to do first.getLastName().length() it would tell me length() did not exist. So, in order to get back the length method I removed the const.

Sorry I must be a bit tired :+( I forgot to say that const functions can only call other const functions (makes sense really) , so all of your get functions should be const too.

And yes, first.getLastName().length() doesn't exist, a function cannot contain another function. Any way just use lexicographical_compare. If you can't use this algorithm for the assignment, then assign the value of the lengths to different variables. It better to assign values outside the loop so those get functions aren't called repeatedly. You only need to loop on the shorter one: if it gets to the end of that, then the shorter one is less than.

This bit could be improved: there are only 2 ways it can go, no need to repeat the logic in reverse.

if (tolower(first.getLastName()[i])<tolower(second.getLastName()[i])) return true;
			else if (tolower(first.getLastName()[i])>tolower(second.getLastName()[i])) return false;

Edit: Actually that part could use the ternary operator on 1 line of code

http://en.cppreference.com/w/cpp/language/operator_other

[TheIdeasMan]

I do need to go to bed now, but there are plenty of others who can help, better than a back yard basher like me :+)

ZZZZZZZZZZZZZZZzzzzzzzzzzzzzzz.....................

[JLBorges]

> I come from C# and we don't have a vector concept and all generics are based really off lists.

CLRs System.Collections.Generic.List<T> is similar to std::vector<T> where the type T is a CTS value type
System.Collections.Generic.List<T> is similar to std::vector< std::shared_ptr<T> > where the type T is a CTS reference type

CLRs System.Collections.Generic.LinkedList<T> is similar to std::list<T> where the type T is CTS a value type
System.Collections.Generic.LinkedList<T> is similar to std::list< std::shared_ptr<T> > where the type T is a CTS reference type

Even if the syntax looks somewhat similar, there are many fundamental differences between C# and C++. This may give you an idea of some of them:

person.h

// see: https://en.wikipedia.org/wiki/Include_guard

#ifndef PERSON_H_INCLUDED
#define PERSON_H_INCLUDED

#include <string>

namespace test
{
    struct person
    {
        // explicitly defaulted default constructor:
        // initialises the three name components to empty strings
        // see: http://www.stroustrup.com/C++11FAQ.html#default
        person() = default ;

        // member initializer list
        // see: http://en.cppreference.com/w/cpp/language/initializer_list
        // move semantics see: http://www.stroustrup.com/C++11FAQ.html#rval
        person( std::string first_name, std::string middle_name, std::string last_name )
            : first_name( std::move(first_name) ),
              middle_name( std::move(middle_name) ),
              last_name( std::move(last_name) ) {}

        // other foundation operations are implicitly defaulted
        // see rule of zero: http://en.cppreference.com/w/cpp/language/rule_of_three

        // const see: https://isocpp.org/wiki/faq/const-correctness#const-member-fns
        std::string full_name() const ;

        // ...

        // https://isocpp.org/wiki/faq/const-correctness
        // lexicographically compare last names ignoring case
        // return true if the this->last_name compares less than that.last_name
        bool cmp_last_name( const person& that ) const ;

        private:
            std::string first_name ;
            std::string middle_name ;
            std::string last_name ;
    };

    // make the type person LessThanComparable
    // see: http://en.cppreference.com/w/cpp/concept/LessThanComparable
    // inline see: http://en.cppreference.com/w/cpp/language/inline
    // koenig lookup see: http://en.cppreference.com/w/cpp/language/adl
    inline bool operator< ( const person& a, const person& b ) { return a.cmp_last_name(b) ; }
}

#endif // PERSON_H_INCLUDED 

person.cpp

#include "person.h"
#include <cctype>

namespace test
{
    std::string person::full_name() const
    {
        return last_name + ", " + first_name + ' ' + middle_name ;
    }

    bool person::cmp_last_name( const person& that ) const
    {
        // lambda expression: http://www.stroustrup.com/C++11FAQ.html#lambda
        // auto: http://www.stroustrup.com/C++11FAQ.html#auto
        static const auto to_lower = [] ( std::string str )
        {
            // range-based for: http://www.stroustrup.com/C++11FAQ.html#for
            for( char& c : str ) c = std::tolower(c) ;
            return str ;
        };

        // http://en.cppreference.com/w/cpp/string/basic_string/operator_cmp
        return to_lower(last_name) < to_lower( that.last_name ) ;
    }
}

main.cpp

#include "person.h"
#include <vector>
#include <algorithm>
#include <iostream>

int main()
{
    // https://cal-linux.com/tutorials/vectors.html
    // http://en.cppreference.com/w/cpp/language/list_initialization
    std::vector<test::person> people
    {
        // http://www.stroustrup.com/C++11FAQ.html#uniform-init
        { "Dennis", "MacAlistair", "Ritchie" },
        { "Kenneth", "Lane", "Thompson" },
        { "Bjarne", "", "Stroustrup" },
        { "Alexander", "Alexandrovich", "Stepanov" },
        { "Andrew", "Joshua", "Koenig" },
        { "Andrei", "", "Alexandrescu" },
    };

    // https://cal-linux.com/tutorials/STL.html
    // http://en.cppreference.com/w/cpp/algorithm/sort
    // http://en.cppreference.com/w/cpp/iterator/begin
    // http://en.cppreference.com/w/cpp/iterator/end
    std::sort( std::begin(people), std::end(people) ) ;

    for( const auto& per : people ) std::cout << per.full_name() << '\n' ;
}

http://coliru.stacked-crooked.com/a/a6a36a324d9c3d0f
http://rextester.com/IGBME62360

[dbag]

Thank you so much @JLBorges for the in depth explanation. You have no idea how much this help me!