map.find() not working

This forum can be used to talk about general design strategies, new ideas and questions in general related to wxWidgets. If you feel your questions doesn't fit anywhere, put it here.
Post Reply
alvindera97
Earned a small fee
Earned a small fee
Posts: 20
Joined: Sun Apr 12, 2020 6:11 pm

map.find() not working

Post by alvindera97 »

Good day everyone. It's me again. I hope we you are all doing well. I have come to a point in my wxWidgets application where user input on wxTextCtrl needs to be evaluated against a map of keys and values (python's dictionary equivalent). If such entry in the wxTextCtrl is found in the dictionary, it's corresponding data is evaluated and post processed (e.g. employed in a mathematical calculation) to produce a final result that will be displayed via a wxMessageBox,

Consider this event handler's code:

Code: Select all

void MyFrame::OnOneHundredLevelDisplayGpButtonClicked(wxCommandEvent& event) {
    using namespace std;

    wxString course_check = wxT("MEE211");
    wxString course_check_2 = wxT("MEE212");

    std::map<wxString, int> first_courses;
    first_courses["MEE211"] = 3;
    first_courses["MEE212"] = 2;
    if (first_courses.find(course_one->GetValue())) {
        wxMessageBox(course_one->GetValue());
    }

    event.Skip();

In the event handler (above), I declared and created 2 wxString objects: "MEE211" and "MEE212". These were inserted into a map of wxStrings and int. The idea goes thus:

When data is entered into wxTextCtrl course_one; and the value is either MEE211 or MEE212 and the Calculate Gp button is clicked, a wxMessageBox is presented displaying the text that was entered. Basically there is a search done within the map to check if the text entered into course_one exists in the map.

Eventually, I would like to ascertain the value of the entered text (in the case of MEE211 it's "3") and use 3 in a mathematical formula to get to a result.

As the code is compiled, I get a compilation error saying:

Code: Select all

hello_world.cpp: In member function ‘void MyFrame::OnOneHundredLevelDisplayGpButtonClicked(wxCommandEvent&)’:
hello_world.cpp:1287:27: error: could not convert ‘first_courses.std::map<wxString, int>::find(wxTextCtrl::GetValue())’ from ‘std::map<wxString, int>::iterator {aka std::_Rb_tree_iterator<std::pair<const wxString, int> >}’ to ‘bool’
     if (first_courses.find(course_one->GetValue())) {
From the error, I can understand that to some extent, the is no real support for the conversion it's trying to do. However, I would like to know if this is possible with wxWidgets. I have primary and extensive knowledge with python. This would have been possible using python's Dictionary data type. I don't know if using the map context is possible in wxWidgets and if it's possible, please educate me. Thank you!!!
PB
Part Of The Furniture
Part Of The Furniture
Posts: 4193
Joined: Sun Jan 03, 2010 5:45 pm

Re: map.find() not working

Post by PB »

I believe that std::map::find() returns an iterator which cannot be implicitly converted to a bool.

You should try changing the comparison to

Code: Select all

if (first_courses.find(course_one->GetValue()) != first_courses.end())
BTW, this has nothing do with wxWidgets, this is just C++ standard library, you can check the docs for find() e.g. here
https://en.cppreference.com/w/cpp/container/map/find

BTW2, I wonder if wxChoice should not be used here instead of wxTextCtrl, where the course id would be stored as client data. This could make both the UI and code more robust...

BTW3, in case you wonder how to get the value based on key, it would be something like

Code: Select all

const auto iterator = first_courses.find(key);

if ( iterator != first_courses.end() ) 
{
     wxLogMessage("Value for key '%s' is %d.", iterator->first, iterator->second);
}
I.e., iterator->first contains the key while iterator->second has the value.
ONEEYEMAN
Part Of The Furniture
Part Of The Furniture
Posts: 7459
Joined: Sat Apr 16, 2005 7:22 am
Location: USA, Ukraine

Re: map.find() not working

Post by ONEEYEMAN »

Hi,
Yes - text cobtrol is definitely is not a tool for a job. wxComboBox would be much better.

Thank you.
alvindera97
Earned a small fee
Earned a small fee
Posts: 20
Joined: Sun Apr 12, 2020 6:11 pm

Re: map.find() not working

Post by alvindera97 »

Thank you... After making research into wxComboBox, I understood the reason for your suggestion. You are right. However, the list of variables I have is well above 30. I wouldn't want my users to have to scroll through 20+ options to get to where they want to. This selection would happen about 17 times per session. There are 5 sessions in total. So that's a lot of work for the users. Thank you for your input. I really appreciate.

I come from the humble beginnings of python and Python dictionaries have always seemed like the most practical solution to this problem hence the preference of C++ maps.
alvindera97
Earned a small fee
Earned a small fee
Posts: 20
Joined: Sun Apr 12, 2020 6:11 pm

Re: map.find() not working

Post by alvindera97 »

PB wrote: Sun May 03, 2020 8:47 pm I believe that std::map::find() returns an iterator which cannot be implicitly converted to a bool.

You should try changing the comparison to

Code: Select all

if (first_courses.find(course_one->GetValue()) != first_courses.end())
BTW, this has nothing do with wxWidgets, this is just C++ standard library, you can check the docs for find() e.g. here
https://en.cppreference.com/w/cpp/container/map/find

BTW2, I wonder if wxChoice should not be used here instead of wxTextCtrl, where the course id would be stored as client data. This could make both the UI and code more robust...

BTW3, in case you wonder how to get the value based on key, it would be something like

Code: Select all

const auto iterator = first_courses.find(key);

if ( iterator != first_courses.end() ) 
{
     wxLogMessage("Value for key '%s' is %d.", iterator->first, iterator->second);
}
I.e., iterator->first contains the key while iterator->second has the value.
Thank you for your explanation of the subject matter and how to more efficiently query my map to get the preferred results. It went a long way to making me more confident on the topic of maps in C++. However, It is sad that I could not define my map in my constructor.
ONEEYEMAN
Part Of The Furniture
Part Of The Furniture
Posts: 7459
Joined: Sat Apr 16, 2005 7:22 am
Location: USA, Ukraine

Re: map.find() not working

Post by ONEEYEMAN »

Hi,
You can successfully divide you choices selectons in groups and make couple of wxComboBox/es.
This solution will be much better than pushing the user of your software to enter the text that will not be guaranteed.

The solution I'm proposing is much better, more robust and will simplify the life of you and the users of your program.

Thank you.
PB
Part Of The Furniture
Part Of The Furniture
Posts: 4193
Joined: Sun Jan 03, 2010 5:45 pm

Re: map.find() not working

Post by PB »

alvindera97 wrote: Wed May 06, 2020 12:56 am I wouldn't want my users to have to scroll through 20+ options to get to where they want to.
BTW, I assume that you are aware that wxTextCtrl has AutoComplete() method which could assist users in typing their option? Of course, if the options are short and they differ only at their ends (ABCDE1, ABCDE2,...), this would not be helpful much.

Edit, this is a simplified example of how the task could be handled in wxWidgets

Code: Select all

#include <wx/wx.h>

#include <map>

enum class CourseId
{
    NotSelected = 0,
    Biol1       = 10,
    Biol2       = 11,
    Engl1       = 20,
    Engl2       = 21,
    Math1       = 30,
    Math2       = 32,
};

typedef std::map<wxString, CourseId> Courses;

bool LoadCourses(Courses& courses)
{ // in a real program, courses would be loaded from a database

    courses.clear();

    courses["BIOL1"] = CourseId::Biol1;
    courses["BIOL2"] = CourseId::Biol2;
    courses["ENGL3"] = CourseId::Engl1;
    courses["ENGL2"] = CourseId::Engl2;
    courses["MATH1"] = CourseId::Math1;
    courses["MATH2"] = CourseId::Math2;

    return true;
}

class MyDialog : public wxDialog
{
public:
    MyDialog(const Courses& courses): 
        wxDialog(nullptr, wxID_ANY, "Select courses"),
        m_courses(courses)
    {
        wxASSERT(!m_courses.empty());
        
        wxBoxSizer* mainSizer = new wxBoxSizer(wxVERTICAL);
        wxArrayString courseNames;
        wxTextValidator courseValidator(wxFILTER_ALPHANUMERIC | wxFILTER_INCLUDE_LIST);

        // add course names for autocompleting and validation
        for ( const auto i : m_courses )
            courseNames.push_back(i.first);

        mainSizer->Add(new wxStaticText(this, wxID_ANY, "Course 1"), wxSizerFlags().Border(wxALL & ~wxBOTTOM));
        m_ctlCourse1NameEdt = new wxTextCtrl(this, wxID_ANY);
        courseValidator.SetIncludes(courseNames);
        m_ctlCourse1NameEdt->SetValidator(courseValidator);
        m_ctlCourse1NameEdt->SetHint("Enter course name");
        m_ctlCourse1NameEdt->AutoComplete(courseNames);
        mainSizer->Add(m_ctlCourse1NameEdt, wxSizerFlags().Expand().Border(wxALL & ~wxTOP));

        mainSizer->Add(CreateStdDialogButtonSizer(wxOK | wxCANCEL), wxSizerFlags().DoubleBorder());

        SetSizerAndFit(mainSizer);
    }

    CourseId GetSelectedCourseId() const
    {
         const auto iterator = m_courses.find(m_ctlCourse1NameEdt->GetValue().Upper());

        if ( iterator != m_courses.end() )
            return iterator->second;
        else
            return CourseId::NotSelected;
    }

private:
    const Courses& m_courses;
    wxTextCtrl* m_ctlCourse1NameEdt;
};

class MyApp : public wxApp
{
public:
    bool OnInit() override
    {
        Courses courses;

        if ( !LoadCourses(courses) )
            wxLogError("Could not load the courses.");
        else
        {
            MyDialog dlg(courses);

             if ( dlg.ShowModal() == wxID_OK )
                 wxLogMessage("Selected course ID: %d", static_cast<int>(dlg.GetSelectedCourseId()));

        }
        return false;
    }
}; wxIMPLEMENT_APP(MyApp);
However, this is still rather crude. In a real application the programmer must handle case (in)sensitivity, accidental spaces etc. (a program should be quite flexible handling this in order not to annoy its users); the generic message shown by wxTextValidator when the string is not in the includes list would be also unacceptable. Custom validation dealing with all this would need to be implemented (not that difficult though).

I.e., properly handling arbitrary user input is much more work than just getting a string from wxChoice.
alvindera97
Earned a small fee
Earned a small fee
Posts: 20
Joined: Sun Apr 12, 2020 6:11 pm

Re: map.find() not working

Post by alvindera97 »

I would like to thank everyone on the platform who have contributed to this question. For me, I have loads of years of experience with python - whom is far less verbose when compared to C++. After reading the comments and suggestions concerning this question, I realized that my problem was basically my general knowledge of C++ and not a wxWidgets related problem.

After a few more digging, I knew I was defining my map within the constructor and that is not supported in C++. From what I could research, I found out that maps more comfortably live within a function. The other options for declaring maps were too complex for my current understanding of C++. So I decided to duplicate my map in every function (or event handler) that it was required. The application is working perfectly now and complete [although it is well above 3000 lines of code].

I don't particularly know which to tick as an answer to this question as all the suggestions are brilliant when analyzed more broadly. Thanks again anyone. However, I will mark this comment as the answer :| . For future researchers, my solution might not be the solution to theirs but the thread would read [SOLVED] (or answered) and hence, better ideas can be generated from the brilliant suggestions posted here.

Thanks again everyone! I am indeed grateful! :D
PB
Part Of The Furniture
Part Of The Furniture
Posts: 4193
Joined: Sun Jan 03, 2010 5:45 pm

Re: map.find() not working

Post by PB »

alvindera97 wrote: Mon May 11, 2020 6:49 amThe other options for declaring maps were too complex for my current understanding of C++.
I do not think it is that complex. ;)

Code: Select all

// declaration of a new type called Courses
typedef std::map<wxString, int> Courses;

// declaration of variable of type Courses
Courses myCourses;
alvindera97
Earned a small fee
Earned a small fee
Posts: 20
Joined: Sun Apr 12, 2020 6:11 pm

Re: map.find() not working

Post by alvindera97 »

PB wrote: Mon May 11, 2020 7:12 am
alvindera97 wrote: Mon May 11, 2020 6:49 amThe other options for declaring maps were too complex for my current understanding of C++.
I do not think it is that complex. ;)

Code: Select all

// declaration of a new type called Courses
typedef std::map<wxString, int> Courses;

// declaration of variable of type Courses
Courses myCourses;

Yes... You are absolutely correct. However, my intention was to create a map of all the courses once (most suitably as a variable) and have my event handlers check that map whenever the credit load of a course had to evaluated. I couldn't do that in the constructor. This is what I tried (in the constructor):

Code: Select all

using namespace std;
std::map<wxString, float> all_courses_dictionary;
That didn't work. And I cannot really remember what compilation error was.
Last edited by alvindera97 on Mon May 11, 2020 9:18 am, edited 1 time in total.
PB
Part Of The Furniture
Part Of The Furniture
Posts: 4193
Joined: Sun Jan 03, 2010 5:45 pm

Re: map.find() not working

Post by PB »

alvindera97 wrote: Mon May 11, 2020 7:24 am Yes... You are absolutely correct. However, my intention was to create a map of all the courses once (most suitably as a variable) and have my event handlers check that map whenever the credit load of a course had to evaluated.This is what I tried (in the constructor):

Code: Select all

using namespace std;
std::map<wxString, float> all_courses_dictionary>;
You meant you wanted to create a global variable. You needed to declare it in a header file, define it in a source file, and use it in the code.

I assume the extraneous ">" before the semicolon is a typo.
alvindera97
Earned a small fee
Earned a small fee
Posts: 20
Joined: Sun Apr 12, 2020 6:11 pm

Re: map.find() not working

Post by alvindera97 »

PB wrote: Mon May 11, 2020 8:37 am
alvindera97 wrote: Mon May 11, 2020 7:24 am Yes... You are absolutely correct. However, my intention was to create a map of all the courses once (most suitably as a variable) and have my event handlers check that map whenever the credit load of a course had to evaluated.This is what I tried (in the constructor):

Code: Select all

using namespace std;
std::map<wxString, float> all_courses_dictionary>;
You meant you wanted to create a global variable. You needed to declare it in a header file, define it in a source file, and use it in the code.

I assume the extraneous ">" before the semicolon is a typo.
Yes.. it was an error (I've corrected it). Thank you for breaking down the process to declare a global variable. I'll look into it. :D
PB
Part Of The Furniture
Part Of The Furniture
Posts: 4193
Joined: Sun Jan 03, 2010 5:45 pm

Re: map.find() not working

Post by PB »

If the code is not a throwaway, I think the correct way would be to abstract and encapsulate the courses data in a class.

It takes more code but is IMO more maintainable, e.g.
// stores courses as id and name of the course

Code: Select all

class Courses
{
public:
    bool Load();
    bool IsLoaded() const;
    
    bool HasCourseWithName(const wxString& name) const;
    int GetCourseId(const wxString& name) const;
    wxArrayString GetCoursesNames() const;

    static Courses& Get();
private:
    std::map<wxString, int> m_data;
};

bool Courses::Load()
{
    wxCHECK_RET(!m_data.empty(), "Courses already loaded", false);
    
    // load data

    return true;
}

bool Courses::IsLoaded() const
{
    return !m_data.empty();
}

bool Courses::HasCourseWithName(const wxString& name) const
{
    return m_data.find(name) != m_data.end();
}

int Courses::GetCourseId(const wxString& name) const
{
    const auto it = m_data.find(name);

    if ( it != m_data.end() )
        return it->second;
    
    return -1;
}
wxArrayString Courses::GetCoursesNames() const
{
    wxArrayString names;
    
    for ( const auto& course : m_data )
        names.push_back(course.first);

    return names;
}

Courses& Courses::Get()
{
    static Courses globalInstance;

    return globalInstance;
}
And once in the code load the courses from the database (or wherever)

Code: Select all

       
Courses courses = Courses::Get();

if ( !courses.Load() )
{
    // handle the error
}
And use it as read only elsewhere, e.g.

Code: Select all

const Courses courses = Courses::Get();
const wxArrayString coursesNames = courses.GetCoursesNames();
Post Reply