Skip to content

Commit

Permalink
C++ - use more expressive std::any_of rather than std::find
Browse files Browse the repository at this point in the history 
What's looked for is encapsulated in a lambda function
named is_req_column_type. This compares values in the vector
with a value captured from the main context, which is updated
for each call of std::any.
  • Loading branch information
@gjanssens
gjanssens committed Mar 13, 2023
1 parent 0ed0be4 commit 16cc218
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions gnucash/import-export/csv-imp/assistant-csv-price-import.cpp
View file
Expand Up @@ -1683,31 +1683,31 @@ void CsvImpPriceAssist::preview_refresh_table ()
preview_style_column (i, combostore);

auto column_types = price_imp->column_types_price();
GncPricePropType req_column_type;
auto is_req_column_type = [&req_column_type] (GncPricePropType column_type)->bool
{ return column_type == req_column_type; };

// look for a namespace column, clear the commodity combo
auto col_type_name = std::find (column_types.begin(),
column_types.end(), GncPricePropType::FROM_NAMESPACE);
if (col_type_name != column_types.end())
req_column_type = GncPricePropType::FROM_NAMESPACE; // Used by is_column_type()
if (std::any_of (column_types.begin(), column_types.end(), is_req_column_type))
{
g_signal_handlers_block_by_func (commodity_selector, (gpointer) csv_price_imp_preview_commodity_sel_cb, this);
set_commodity_for_combo (GTK_COMBO_BOX(commodity_selector), nullptr);
g_signal_handlers_unblock_by_func (commodity_selector, (gpointer) csv_price_imp_preview_commodity_sel_cb, this);
}

// look for a symbol column, clear the commodity combo
auto col_type_sym = std::find (column_types.begin(),
column_types.end(), GncPricePropType::FROM_SYMBOL);
if (col_type_sym != column_types.end())
req_column_type = GncPricePropType::FROM_SYMBOL; // Used by is_column_type()
if (std::any_of (column_types.begin(), column_types.end(), is_req_column_type))
{
g_signal_handlers_block_by_func (commodity_selector, (gpointer) csv_price_imp_preview_commodity_sel_cb, this);
set_commodity_for_combo (GTK_COMBO_BOX(commodity_selector), nullptr);
g_signal_handlers_unblock_by_func (commodity_selector, (gpointer) csv_price_imp_preview_commodity_sel_cb, this);
}

// look for a currency column, clear the currency combo
auto col_type_curr = std::find (column_types.begin(),
column_types.end(), GncPricePropType::TO_CURRENCY);
if (col_type_curr != column_types.end())
req_column_type = GncPricePropType::TO_CURRENCY; // Used by is_column_type()
if (std::any_of (column_types.begin(), column_types.end(), is_req_column_type))
{
g_signal_handlers_block_by_func (currency_selector, (gpointer) csv_price_imp_preview_currency_sel_cb, this);
set_commodity_for_combo (GTK_COMBO_BOX(currency_selector), nullptr);
Expand Down

5 comments on commit 16cc218

@christopherlam
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a functional programming pov I get nervous with the varying req_column_type! I'd write as:

auto any_of_type = [](std::vector<GncPricePropType>& column_types, GncPricePropType req_column_type) -> bool
{
    return std::any_of (column_types.begin(), column_types.end(), 
                        [&req_column_type](GncPricePropType column_type)->bool
                        { return column_type == req_column_type; });
}

if (any_of_type (column_types, GncPricePropType::FROM_SYMBOL))
if (any_of_type (column_types, GncPricePropType::TO_CURRENCY))

@gjanssens
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that looks better in this case. Feel free to submit a fix.

@jralls
Copy link
Member

@jralls jralls commented on 16cc218 Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the wrapper lambda there's no reason to capture req_column_type by reference in the inner lambda.

@gjanssens
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it was never needed to capture by reference. But for clarity, we still have to capture it, no ? Is there another way to pass it to the inner lambda ?

@jralls
Copy link
Member

@jralls jralls commented on 16cc218 Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, as it is now it does need to capture by reference. A lambda is a closure and the capture happens at the point that the lambda is created, in this case when it's assigned to is_req_column_type and if it captured req_column_type by value it would get whatever garbage happened to be there.

Please sign in to comment.