C++ find_if member variable of another class

Question

So I have a class called Song, and another class Called SongLibrary. The Songlibrary just contains a set of all songs and appropriate methods.

I am currently trying to make a function to search the song library and check if a song has a particular title.

The problem I am having is that the song title is inaccessible from the songlibrary class.

m_songs is the name of the set I am using in songlibrary to store all the songs.

m_title is the member variable for title in Song.cpp

in SongLibrary.cpp

bool SongLibrary::SearchSong(string title)
{
    bool found = false;

    std::find_if(begin(m_songs), end(m_songs),
        [&](Song const& p) 
    { 
        if (p.m_title == title) // error here (m_title is inaccessible)
        {
            found = true;
        }
    });

    return found;

}

I have attempted to make the method a friend of the song class but i am not exactly sure I understand how it works.

EDIT I Fixed the problem using the following

bool SongLibrary::SearchSong(string title)
{
    if (find_if(begin(m_songs), end(m_songs),[&](Song const& p)

    {return p.getTitle() == title;}) != end(m_songs))
    {
        return true;
    }
    return false;

}

Make `SongLibrary` a friend of `Song`. Or better yet, make `m_title` public. — Hatted Rooster, Nov 06 '17 at 14:54
Why does your `Song` class not have a `public` way to get the title? — nwp, Nov 06 '17 at 14:54
That's not your only problem. The call to find_if above should not compile because the supplied predicate does not return a value. — Richard Hodges, Nov 06 '17 at 15:16
@RichardHodges Yes bit then how do I return from the SearchSong function? — Michael Grinnell, Nov 06 '17 at 15:24
@MichaelGrinnell `find_if` returns `end(m_songs)` if nothing is found. That should be what you use determine the return value of `SearchSong`. — Chris Drew, Nov 06 '17 at 15:26
@RichardHodges I thought it returned a bool. Thank you so much, I got it working. — Michael Grinnell, Nov 06 '17 at 15:32

score 1 · Accepted Answer · edited Nov 06 '17 at 15:13

1

If you want to use friend classes, you should make SongLibrary a friend of Song. But I suggest you make a public getter for your Song-title like this:

const std::string& getTitle() const { return m_title; }

edited Nov 06 '17 at 15:13

Jarod42

203,559
14
181
302

answered Nov 06 '17 at 14:56

Jasper

99
9

Except having a public getter for only returning a member is an anti-pattern. – Hatted Rooster Nov 06 '17 at 14:57
1

should be `const`, and return const reference to avoid copies. – Jarod42 Nov 06 '17 at 14:57
@RickAstley: It would be debatable if return type were non-const reference, but it is not. – Jarod42 Nov 06 '17 at 14:59
@Jarod42 Not really, I'll agree it's very personal but offering "encapsulation" by privating the member and then adding public get and setters is a code smell to me. – Hatted Rooster Nov 06 '17 at 15:01
yeah I agree it should be a const ref. I updated my answer – Jasper Nov 06 '17 at 15:02
Should all getters and setters be const ref? – Michael Grinnell Nov 06 '17 at 15:04
@Jarod42 Agree the member function should be `const` but [I was taught](https://www.amazon.co.uk/Effective-Specific-Programs-Professional-Computing/dp/0321334876) to not return reference to internals, even a const reference. So I think returning by-value is a reasonable start and returning by reference is an optimization. – Chris Drew Nov 06 '17 at 15:05
Also I believe you only need const after not before – Michael Grinnell Nov 06 '17 at 15:13
@ChrisDrew: Returning by const ref instead of value is indeed an optimization and in rare cases, behavior can change (keeping reference of return value in member for example (as for regular variable there is life time extension)). Some libraries expose in doc return by value and explicit state that return by reference can be used instead. – Jarod42 Nov 06 '17 at 15:25
@Jarod42 Sure. The classic failure case of getter returning const reference is: `const std::string& title = funcThatReturnsASongByValue().getTitle();` ... oops! But we are probably getting off-topic. – Chris Drew Nov 06 '17 at 15:42

score 0 · Answer 2 · answered Nov 06 '17 at 14:57

0

Add a "getter" function in song, for example

class Song {
    public: 
       const std::string& getTitle(){
           return Title;
       }
       ...
    private:
       ...
       std::string Title;
}

answered Nov 06 '17 at 14:57

DingusKhan

123
1
15

C++ find_if member variable of another class

2 Answers2