gansm / finalcut

A text-based widget toolkit
https://github.com/gansm/finalcut/wiki/First-steps#first-steps-with-the-final-cut-widget-toolkit
GNU Lesser General Public License v3.0
993 stars 52 forks source link

FListView crashing #113

Closed wimstockman closed 1 year ago

wimstockman commented 1 year ago

FListView Crashing if no setColumnSort is given

Hi Markus, I don't know if it is a bug or a feature but the setColumnSort seems mandatory on the FListView. I couldn't find it in some documentation.

How to reproduce the bug?

Here under is a small piece of the code that create dynamically the columns if i leave the column sort out. it crashes.


void DataView::populate(finalcut::FString sTable)
{
    this->setText(sTable);
    this->listview.clear();
    std::unique_ptr<sql::Connection> conn;
    conn=connect_db();
    try
    {
        finalcut::FString SQL{"DESCRIBE  "};
        SQL << sTable <<";";
        sql::ResultSet* res = query_db(SQL);
        while (res->next())
        {
            listview.addColumn(res->getString(1).c_str());

        }
    listview.setColumnSortType (1, finalcut::SortType::Name);
    listview.setColumnSort (1, finalcut::SortOrder::Descending);
    listview.hideSortIndicator(false);
        SQL ="SELECT * FROM ";
        SQL << sTable << " ORDER BY DATE DESC LIMIT 150;";
        sql::ResultSet* rs = query_db(SQL);
        sql::ResultSetMetaData *rsmd=rs->getMetaData();
        int fieldcount = rsmd->getColumnCount();
        while (rs->next())
        {   
            std::vector<std::string> v;
            for (int i=1 ; i<= fieldcount; i++) v.push_back(rs->getString(i).c_str());
            const finalcut::FStringList line (v.cbegin(),v.cend());
            this->listview.insert (line);
        }
    }
    catch(sql::SQLException& e)
    {
        std::cerr<< "Error" << e.what() << std::endl;
    }   

}

What behavior did you expect?

Just no sort :-)

Thanks for the great project !!!

In which environment did the bug occur?

gansm commented 1 year ago

Indeed, I have found an invalid iterator after inserting an item in FListView. It was the internal first_visible_line iterator that became invalid after the list was reallocated (adf6200).

A FListView widget initially has no sorting without explicit specification. Unless you force this by clicking on a column header or with setColumnSort(1, ...).

grafik

grafik

Here is my test code:

#include <final/final.h>

using namespace finalcut;

class ListDialogWidget : public FDialog
{
  public:
    explicit ListDialogWidget (FWidget* parent = nullptr)
      : FDialog{parent}
    {
      quit_btn.addCallback
      (
        "clicked",
        finalcut::getFApplication(),
        &finalcut::FApplication::cb_exitApp,
        this
      );

      clear_btn.addCallback ("clicked", this, &ListDialogWidget::cb_clear);
      populate_btn.addCallback ("clicked", this, &ListDialogWidget::cb_populate);
      initListView();
      populateListView();
    }

  private:
    void initLayout()
    {
      setText ("Unsorted list view");
      setGeometry (FPoint{15, 5}, FSize{50, 18});
      populate_btn.setGeometry (FPoint{2, 14}, FSize{14, 1});
      clear_btn.setGeometry (FPoint{18, 14}, FSize{11, 1});
      quit_btn.setGeometry (FPoint{37, 14}, FSize{10, 1});
      listview.setGeometry(FPoint{2, 1}, FSize{46, 12});
      FDialog::initLayout();
    }

    void initListView()
    {
      listview.addColumn ("Username", 10);
      listview.addColumn ("ID", 6);
      listview.addColumn ("First name", 12);
      listview.addColumn ("Last name", 12);
    }

    void populateListView()
    {
      struct Item
      {
        const char* username;
        const char* id;
        const char* first_name;
        const char* last_name;
      };

      auto data = std::initializer_list<Item> \
      {
        {"smith79", "5079", "Jamie", "Smith"},
        {"grey07", "2070", "Laura", "Grey"},
        {"johnson81", "4081", "Craig", "Johnson"},
        {"booker12", "9012", "Rachel", "Booker"},
        {"jenkins46", "9346", "Mary", "Jenkins"}
      };

      for (auto data_line : data)
      {
        FStringList line ( &data_line.username
                         , &data_line.last_name + 1 );
        listview.insert(line);
      }
    }

    void cb_clear()
    {
      listview.clear();
    }

    void cb_populate()
    {
      populateListView();
      listview.redraw();
    }

    // Data members
    FListView listview{this};
    FButton populate_btn{"&Populate", this};
    FButton clear_btn{"&Clear", this};
    FButton quit_btn{"&Quit", this};
};

int main (int argc, char* argv[])
{
  FApplication app(argc, argv);
  ListDialogWidget dialog(&app);
  FWidget::setMainWidget(&dialog);
  dialog.show();
  return app.exec();
}
wimstockman commented 1 year ago

Danke schon. you fixed it :-) Now my table loads instantly. before it took 10 sec just for 4000 rows of data. I guess it was sorting the whole table on each insert again. Vielen dank Wim Stockman

gansm commented 1 year ago

Hello Wim, before inserting 4000 rows, it's recommended to increase the capacity before. It avoids reallocations and copying of the data. It will speed up the insertion of the data rows.

listview.getData().reserve(4000);

Further, it is recommended to pass the return value of rs->getString(i) directly instead of using c_str() to avoid deep data copies, if it is of datatype std::string or std::wstring.

wimstockman commented 1 year ago

Hi , thanks for the suggestion. the datatype is of SQLString so that's why i use c_str(). Kind Regards, Wim stockman

gansm commented 1 year ago

If you use the mysql-connector-cpp, you have access to the internal std::string with the function call operator (operator()).

class SQLString
{
    std::string realStr;

  public:
    ~SQLString() {}

    SQLString() {}

    SQLString(const SQLString & other) : realStr(other.realStr) {}

    SQLString(const std::string & other) : realStr(other) {}

    SQLString(const char other[]) : realStr(other) {}

    SQLString(const char * s, size_t n) : realStr(s, n) {}

    [...]

    // Conversion to std::string. Comes in play for stuff like std::string str= SQLString_var;
    operator const std::string &() const
    {
       return realStr;
    }

    /** For access std::string methods. Not sure we need it. Makes it look like some smart ptr.
        possibly operator* - will look even more like smart ptr */
    std::string * operator ->()
    {
       return & realStr;
    }

    [...]

    const char * c_str() const
    {
      return realStr.c_str();
    }

    [...]
};

Implementation: mysql-connector-cpp/cppconn/sqlstring.h

wimstockman commented 1 year ago

Hi Markus, I'm using the mariadb-connector-cpp. They don't include that function. the only function I can find is std::string(). which returns a c_str().

 class MARIADB_EXPORTED SQLString final {

friend class StringImp;

  std::unique_ptr<StringImp> theString;
public:
  SQLString(const SQLString&);
  SQLString(SQLString&&); //Move constructor
  SQLString(const std::string& str) : SQLString(str.c_str(), str.length()) {}
  SQLString(const char* str);
  SQLString(const char* str, std::size_t count);
  SQLString();
  ~SQLString();

  static constexpr std::size_t npos{static_cast<std::size_t>(-1)};
  const char * c_str() const;
  SQLString& operator=(const SQLString&);
  //operator std::string() { return std::string(this->c_str(), this->length()); }
  operator const char* () const;
  SQLString& operator=(const char * right);
  bool operator <(const SQLString&) const;

implementation: mariadb-connector-cpp/

Thanks Wim

wimstockman commented 1 year ago

Hi , Markus, I forked the mariadb-connector-cpp and added a toString() function which just returns the underlying std::string(). Kind Regards, Wim