From 2c30e204c46b3496c95f591f7f426b326a30ea04 Mon Sep 17 00:00:00 2001 From: abitmore Date: Mon, 22 Mar 2021 16:01:42 +0000 Subject: [PATCH] Fix code smells --- libraries/app/api.cpp | 10 +++---- libraries/app/database_api.cpp | 21 ++++++------- libraries/app/database_api_impl.hxx | 8 ++--- libraries/app/include/graphene/app/api.hpp | 10 +++---- .../app/include/graphene/app/database_api.hpp | 8 ++--- .../chain/include/graphene/chain/config.hpp | 4 ++- .../api_helper_indexes/api_helper_indexes.cpp | 22 +++++++------- .../api_helper_indexes/api_helper_indexes.hpp | 24 +++++++-------- .../wallet/include/graphene/wallet/wallet.hpp | 28 ++++++++--------- libraries/wallet/wallet.cpp | 30 +++++++++++-------- libraries/wallet/wallet_api_impl.hpp | 2 +- libraries/wallet/wallet_asset.cpp | 2 +- libraries/wallet/wallet_results.cpp | 8 ++--- 13 files changed, 92 insertions(+), 85 deletions(-) diff --git a/libraries/app/api.cpp b/libraries/app/api.cpp index 62cb9317e8..ba97a82d62 100644 --- a/libraries/app/api.cpp +++ b/libraries/app/api.cpp @@ -340,7 +340,7 @@ namespace graphene { namespace app { vector history_api::get_account_history( const std::string account_id_or_name, operation_history_id_type stop, - unsigned limit, + uint32_t limit, operation_history_id_type start ) const { FC_ASSERT( _app.chain_database() ); @@ -391,10 +391,10 @@ namespace graphene { namespace app { } vector history_api::get_account_history_operations( const std::string account_id_or_name, - int operation_type, + int64_t operation_type, operation_history_id_type start, operation_history_id_type stop, - unsigned limit ) const + uint32_t limit ) const { FC_ASSERT( _app.chain_database() ); const auto& db = *_app.chain_database(); @@ -437,7 +437,7 @@ namespace graphene { namespace app { vector history_api::get_relative_account_history( const std::string account_id_or_name, uint64_t stop, - unsigned limit, + uint32_t limit, uint64_t start ) const { FC_ASSERT( _app.chain_database() ); @@ -486,7 +486,7 @@ namespace graphene { namespace app { history_operation_detail history_api::get_account_history_by_operations( const std::string account_id_or_name, flat_set operation_types, - uint32_t start, unsigned limit )const + uint32_t start, uint32_t limit )const { const auto configured_limit = _app.get_options().api_limit_get_account_history_by_operations; FC_ASSERT( limit <= configured_limit, diff --git a/libraries/app/database_api.cpp b/libraries/app/database_api.cpp index 16e0ae97c4..7be68e4b4f 100644 --- a/libraries/app/database_api.cpp +++ b/libraries/app/database_api.cpp @@ -90,7 +90,7 @@ database_api_impl::database_api_impl( graphene::chain::database& db, const appli asset_in_liquidity_pools_index = &_db.get_index_type< primary_index< liquidity_pool_index > >() .get_secondary_index(); } - catch( fc::assert_exception& e ) + catch( const fc::assert_exception& ) { asset_in_liquidity_pools_index = nullptr; } @@ -1827,10 +1827,10 @@ vector database_api_impl::get_liquidity_pools_by } vector database_api::get_liquidity_pools_by_one_asset( - std::string asset_symbol_or_id, - optional limit, - optional start_id, - optional with_statistics )const + const std::string& asset_symbol_or_id, + const optional& limit, + const optional& start_id, + const optional& with_statistics )const { return my->get_liquidity_pools_by_one_asset( asset_symbol_or_id, @@ -1840,10 +1840,10 @@ vector database_api::get_liquidity_pools_by_one_ } vector database_api_impl::get_liquidity_pools_by_one_asset( - std::string asset_symbol_or_id, - optional olimit, - optional ostart_id, - optional with_statistics )const + const std::string& asset_symbol_or_id, + const optional& olimit, + const optional& ostart_id, + const optional& with_statistics )const { // api_helper_indexes plugin is required for accessing the secondary index FC_ASSERT( _app_options && _app_options->has_api_helper_indexes_plugin, @@ -1869,7 +1869,8 @@ vector database_api_impl::get_liquidity_pools_by vector results; results.reserve( limit ); - for ( ; itr != pools.end() && results.size() < limit; ++itr ) + auto pools_end = pools.end(); + for ( ; itr != pools_end && results.size() < limit; ++itr ) { results.emplace_back( extend_liquidity_pool( (*itr)(_db), with_stats ) ); } diff --git a/libraries/app/database_api_impl.hxx b/libraries/app/database_api_impl.hxx index 4aff5ccb33..2088045245 100644 --- a/libraries/app/database_api_impl.hxx +++ b/libraries/app/database_api_impl.hxx @@ -154,10 +154,10 @@ class database_api_impl : public std::enable_shared_from_this optional start_id = optional(), optional with_statistics = false )const; vector get_liquidity_pools_by_one_asset( - std::string asset_symbol_or_id, - optional limit = 101, - optional start_id = optional(), - optional with_statistics = false )const; + const std::string& asset_symbol_or_id, + const optional& limit = 101, + const optional& start_id = optional(), + const optional& with_statistics = false )const; vector get_liquidity_pools_by_both_assets( std::string asset_symbol_or_id_a, std::string asset_symbol_or_id_b, diff --git a/libraries/app/include/graphene/app/api.hpp b/libraries/app/include/graphene/app/api.hpp index af2dc3c1fd..acc82c350b 100644 --- a/libraries/app/include/graphene/app/api.hpp +++ b/libraries/app/include/graphene/app/api.hpp @@ -136,7 +136,7 @@ namespace graphene { namespace app { vector get_account_history( const std::string account_name_or_id, operation_history_id_type stop = operation_history_id_type(), - unsigned limit = 100, + uint32_t limit = 100, operation_history_id_type start = operation_history_id_type() )const; @@ -153,7 +153,7 @@ namespace graphene { namespace app { const std::string account_name_or_id, flat_set operation_types, uint32_t start, - unsigned limit + uint32_t limit )const; /** @@ -168,10 +168,10 @@ namespace graphene { namespace app { */ vector get_account_history_operations( const std::string account_name_or_id, - int operation_type, + int64_t operation_type, operation_history_id_type start = operation_history_id_type(), operation_history_id_type stop = operation_history_id_type(), - unsigned limit = 100 + uint32_t limit = 100 )const; /** @@ -188,7 +188,7 @@ namespace graphene { namespace app { */ vector get_relative_account_history( const std::string account_name_or_id, uint64_t stop = 0, - unsigned limit = 100, + uint32_t limit = 100, uint64_t start = 0) const; /** diff --git a/libraries/app/include/graphene/app/database_api.hpp b/libraries/app/include/graphene/app/database_api.hpp index dff253c99b..fed25b05d5 100644 --- a/libraries/app/include/graphene/app/database_api.hpp +++ b/libraries/app/include/graphene/app/database_api.hpp @@ -706,10 +706,10 @@ class database_api * 4. can only omit one or more arguments in the end of the list, but not one or more in the middle */ vector get_liquidity_pools_by_one_asset( - std::string asset_symbol_or_id, - optional limit = 101, - optional start_id = optional(), - optional with_statistics = false )const; + const std::string& asset_symbol_or_id, + const optional& limit = 101, + const optional& start_id = optional(), + const optional& with_statistics = false )const; /** * @brief Get a list of liquidity pools by the symbols or IDs of the two assets in the pool diff --git a/libraries/chain/include/graphene/chain/config.hpp b/libraries/chain/include/graphene/chain/config.hpp index 1c85d78aac..7fd7f42cef 100644 --- a/libraries/chain/include/graphene/chain/config.hpp +++ b/libraries/chain/include/graphene/chain/config.hpp @@ -25,12 +25,14 @@ #include +#include + #define GRAPHENE_MIN_UNDO_HISTORY 10 #define GRAPHENE_MAX_UNDO_HISTORY 10000 #define GRAPHENE_MAX_NESTED_OBJECTS (200) -#define GRAPHENE_CURRENT_DB_VERSION "20210222" +const std::string GRAPHENE_CURRENT_DB_VERSION = "20210222"; #define GRAPHENE_RECENTLY_MISSED_COUNT_INCREMENT 4 #define GRAPHENE_RECENTLY_MISSED_COUNT_DECREMENT 3 diff --git a/libraries/plugins/api_helper_indexes/api_helper_indexes.cpp b/libraries/plugins/api_helper_indexes/api_helper_indexes.cpp index 91959bd7a4..9c95c8e4ff 100644 --- a/libraries/plugins/api_helper_indexes/api_helper_indexes.cpp +++ b/libraries/plugins/api_helper_indexes/api_helper_indexes.cpp @@ -49,7 +49,7 @@ void amount_in_collateral_index::object_inserted( const object& objct ) itr->second += o.collateral; } -} FC_CAPTURE_AND_RETHROW( (objct) ); } +} FC_CAPTURE_AND_RETHROW( (objct) ) } void amount_in_collateral_index::object_removed( const object& objct ) { try { @@ -67,53 +67,55 @@ void amount_in_collateral_index::object_removed( const object& objct ) itr->second -= o.collateral; } -} FC_CAPTURE_AND_RETHROW( (objct) ); } +} FC_CAPTURE_AND_RETHROW( (objct) ) } void amount_in_collateral_index::about_to_modify( const object& objct ) { try { object_removed( objct ); -} FC_CAPTURE_AND_RETHROW( (objct) ); } +} FC_CAPTURE_AND_RETHROW( (objct) ) } void amount_in_collateral_index::object_modified( const object& objct ) { try { object_inserted( objct ); -} FC_CAPTURE_AND_RETHROW( (objct) ); } +} FC_CAPTURE_AND_RETHROW( (objct) ) } share_type amount_in_collateral_index::get_amount_in_collateral( const asset_id_type& asst )const { try { auto itr = in_collateral.find( asst ); if( itr == in_collateral.end() ) return 0; return itr->second; -} FC_CAPTURE_AND_RETHROW( (asst) ); } +} FC_CAPTURE_AND_RETHROW( (asst) ) } share_type amount_in_collateral_index::get_backing_collateral( const asset_id_type& asst )const { try { auto itr = backing_collateral.find( asst ); if( itr == backing_collateral.end() ) return 0; return itr->second; -} FC_CAPTURE_AND_RETHROW( (asst) ); } +} FC_CAPTURE_AND_RETHROW( (asst) ) } void asset_in_liquidity_pools_index::object_inserted( const object& objct ) { try { - const liquidity_pool_object& o = static_cast( objct ); + const auto& o = static_cast( objct ); asset_in_pools_map[ o.asset_a ].insert( o.id ); // Note: [] operator will create an entry if not found asset_in_pools_map[ o.asset_b ].insert( o.id ); -} FC_CAPTURE_AND_RETHROW( (objct) ); } +} FC_CAPTURE_AND_RETHROW( (objct) ) } void asset_in_liquidity_pools_index::object_removed( const object& objct ) { try { - const liquidity_pool_object& o = static_cast( objct ); + const auto& o = static_cast( objct ); asset_in_pools_map[ o.asset_a ].erase( o.id ); asset_in_pools_map[ o.asset_b ].erase( o.id ); // Note: do not erase entries with an empty set from the map in order to avoid read/write race conditions -} FC_CAPTURE_AND_RETHROW( (objct) ); } +} FC_CAPTURE_AND_RETHROW( (objct) ) } void asset_in_liquidity_pools_index::about_to_modify( const object& objct ) { + // this secondary index has no interest in the modifications, nothing to do here } void asset_in_liquidity_pools_index::object_modified( const object& objct ) { + // this secondary index has no interest in the modifications, nothing to do here } const flat_set& asset_in_liquidity_pools_index::get_liquidity_pools_by_asset( diff --git a/libraries/plugins/api_helper_indexes/include/graphene/api_helper_indexes/api_helper_indexes.hpp b/libraries/plugins/api_helper_indexes/include/graphene/api_helper_indexes/api_helper_indexes.hpp index 216f85d594..49b79bee90 100644 --- a/libraries/plugins/api_helper_indexes/include/graphene/api_helper_indexes/api_helper_indexes.hpp +++ b/libraries/plugins/api_helper_indexes/include/graphene/api_helper_indexes/api_helper_indexes.hpp @@ -38,10 +38,10 @@ using namespace chain; class amount_in_collateral_index : public secondary_index { public: - virtual void object_inserted( const object& obj ) override; - virtual void object_removed( const object& obj ) override; - virtual void about_to_modify( const object& before ) override; - virtual void object_modified( const object& after ) override; + void object_inserted( const object& obj ) override; + void object_removed( const object& obj ) override; + void about_to_modify( const object& before ) override; + void object_modified( const object& after ) override; share_type get_amount_in_collateral( const asset_id_type& asset )const; share_type get_backing_collateral( const asset_id_type& asset )const; @@ -59,10 +59,10 @@ class amount_in_collateral_index : public secondary_index class asset_in_liquidity_pools_index: public secondary_index { public: - virtual void object_inserted( const object& obj ) override; - virtual void object_removed( const object& obj ) override; - virtual void about_to_modify( const object& before ) override; - virtual void object_modified( const object& after ) override; + void object_inserted( const object& obj ) override; + void object_removed( const object& obj ) override; + void about_to_modify( const object& before ) override; + void object_modified( const object& after ) override; const flat_set& get_liquidity_pools_by_asset( const asset_id_type& a )const; @@ -84,16 +84,16 @@ class api_helper_indexes : public graphene::app::plugin std::string plugin_name()const override; std::string plugin_description()const override; - virtual void plugin_set_program_options( + void plugin_set_program_options( boost::program_options::options_description& cli, boost::program_options::options_description& cfg) override; - virtual void plugin_initialize(const boost::program_options::variables_map& options) override; - virtual void plugin_startup() override; + void plugin_initialize(const boost::program_options::variables_map& options) override; + void plugin_startup() override; friend class detail::api_helper_indexes_impl; - std::unique_ptr my; private: + std::unique_ptr my; amount_in_collateral_index* amount_in_collateral_idx = nullptr; asset_in_liquidity_pools_index* asset_in_liquidity_pools_idx = nullptr; }; diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index eb7b6b6f1d..321f7609a2 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -126,7 +126,7 @@ class wallet_api * @param limit the number of entries to return (starting from the most recent) * @returns a list of \c operation_history_objects */ - vector get_account_history(string account_name_or_id, int limit)const; + vector get_account_history(string account_name_or_id, uint32_t limit)const; /** Returns the relative operations on the named account from start number. * @@ -137,7 +137,7 @@ class wallet_api * @returns a list of \c operation_history_objects */ vector get_relative_account_history( string account_name_or_id, uint32_t stop, - int limit, uint32_t start )const; + uint32_t limit, uint32_t start )const; /** * @brief Fetch all objects relevant to the specified account @@ -243,7 +243,7 @@ class wallet_api */ account_history_operation_detail get_account_history_by_operations( string account_name_or_id, flat_set operation_types, - uint32_t start, int limit); + uint32_t start, uint32_t limit); /** Returns the block chain's rapidly-changing properties. * The returned object contains information that changes every block interval @@ -291,7 +291,7 @@ class wallet_api * @param account_name_or_id the name or ID of the account to look up * @returns the name of the account */ - string get_account_name(string account_name_or_id) const + string get_account_name(const string& account_name_or_id) const { return get_account( account_name_or_id ).name; } /** @@ -299,14 +299,14 @@ class wallet_api * @param asset_symbol_or_id the symbol or ID of an asset to look up * @returns the id of the given asset */ - asset_id_type get_asset_id(string asset_symbol_or_id) const; + asset_id_type get_asset_id(const string& asset_symbol_or_id) const; /** * Lookup the symbol of an asset. * @param asset_symbol_or_id the symbol or ID of an asset to look up * @returns the symbol of the given asset */ - string get_asset_symbol(string asset_symbol_or_id) const + string get_asset_symbol(const string& asset_symbol_or_id) const { return get_asset( asset_symbol_or_id ).symbol; } /** @@ -314,7 +314,7 @@ class wallet_api * @param asset_symbol_or_id the symbol or ID of an asset to look up * @returns the symbol of the given asset */ - string get_asset_name(string asset_symbol_or_id) const + string get_asset_name(const string& asset_symbol_or_id) const { return get_asset_symbol( asset_symbol_or_id ); } /** @@ -734,8 +734,8 @@ class wallet_api * @param brain_key the brain key used for generating the account's private keys * @param account_name the name of the account, must be unique on the blockchain. * Names with only latin letters and at least one vowel are - * premium names and expensive to register; - * names with only consonants, or at least with a digit, a dot or + * premium names and expensive to register. + * Names with only consonants, or at least with a digit, a dot or * a minus sign are cheap. * @param registrar_account the account which will pay the fee to register the user * @param referrer_account the account who is acting as a referrer, and may receive a @@ -782,11 +782,11 @@ class wallet_api * increase with transaction size * @returns the transaction ID (hash) along with the signed transaction transferring funds */ - pair transfer2(string from, - string to, - string amount, - string asset_symbol_or_id, - string memo ) { + pair transfer2(const string& from, + const string& to, + const string& amount, + const string& asset_symbol_or_id, + const string& memo ) { auto trx = transfer( from, to, amount, asset_symbol_or_id, memo, true ); return std::make_pair(trx.id(),trx); } diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 467cb157f9..1413867510 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -276,7 +276,7 @@ signed_transaction wallet_api::htlc_extend ( std::string htlc_id, std::string is return my->htlc_extend(htlc_id, issuer, seconds_to_add, broadcast); } -vector wallet_api::get_account_history(string name, int limit)const +vector wallet_api::get_account_history(string name, uint32_t limit)const { vector result; @@ -298,7 +298,9 @@ vector wallet_api::get_account_history(string name, int limit) } } - int page_limit = skip_first_row ? std::min( 100, limit + 1 ) : std::min( 100, limit ); + uint32_t default_page_size = 100; + uint32_t page_limit = skip_first_row ? std::min( default_page_size, limit + 1 ) + : std::min( default_page_size, limit ); vector current = my->_remote_hist->get_account_history( name, @@ -321,7 +323,7 @@ vector wallet_api::get_account_history(string name, int limit) result.push_back( operation_detail{ memo, ss.str(), o } ); } - if( int(current.size()) < page_limit ) + if( current.size() < page_limit ) break; limit -= current.size(); @@ -335,7 +337,7 @@ vector wallet_api::get_account_history(string name, int limit) vector wallet_api::get_relative_account_history( string name, uint32_t stop, - int limit, + uint32_t limit, uint32_t start)const { vector result; @@ -349,22 +351,24 @@ vector wallet_api::get_relative_account_history( else start = std::min(start, stats.total_ops); + uint32_t default_page_size = 100; while( limit > 0 ) { + uint32_t page_size = std::min(default_page_size, limit); vector current = my->_remote_hist->get_relative_account_history( name, stop, - std::min(100, limit), + page_size, start); for (auto &o : current) { std::stringstream ss; auto memo = o.op.visit(detail::operation_printer(ss, *my, o)); result.push_back(operation_detail{memo, ss.str(), o}); } - if (current.size() < std::min(100, limit)) + if (current.size() < page_size) break; - limit -= current.size(); - start -= 100; + limit -= page_size; + start -= page_size; if( start == 0 ) break; } return result; @@ -374,7 +378,7 @@ account_history_operation_detail wallet_api::get_account_history_by_operations( string name, flat_set operation_types, uint32_t start, - int limit) + uint32_t limit) { account_history_operation_detail result; auto account_id = get_account(name).get_id(); @@ -390,10 +394,12 @@ account_history_operation_detail wallet_api::get_account_history_by_operations( result.total_count =stats.removed_ops; } + uint32_t default_page_size = 100; while (limit > 0 && start <= stats.total_ops) { - uint32_t min_limit = std::min (100, limit); + uint32_t min_limit = std::min(default_page_size, limit); auto current = my->_remote_hist->get_account_history_by_operations(name, operation_types, start, min_limit); - for( auto it = current.operation_history_objs.rbegin(); it != current.operation_history_objs.rend(); ++it ) + auto his_rend = current.operation_history_objs.rend(); + for( auto it = current.operation_history_objs.rbegin(); it != his_rend; ++it ) { auto& obj = *it; std::stringstream ss; @@ -591,7 +597,7 @@ account_id_type wallet_api::get_account_id(string account_name_or_id) const return my->get_account_id(account_name_or_id); } -asset_id_type wallet_api::get_asset_id(string asset_symbol_or_id) const +asset_id_type wallet_api::get_asset_id(const string& asset_symbol_or_id) const { return my->get_asset_id(asset_symbol_or_id); } diff --git a/libraries/wallet/wallet_api_impl.hpp b/libraries/wallet/wallet_api_impl.hpp index 2ee1de89d8..663efccaaf 100644 --- a/libraries/wallet/wallet_api_impl.hpp +++ b/libraries/wallet/wallet_api_impl.hpp @@ -169,7 +169,7 @@ class wallet_api_impl fc::optional get_htlc(string htlc_id) const; - asset_id_type get_asset_id(string asset_symbol_or_id) const; + asset_id_type get_asset_id(const string& asset_symbol_or_id) const; string get_wallet_filename() const; diff --git a/libraries/wallet/wallet_asset.cpp b/libraries/wallet/wallet_asset.cpp index a73755750c..80bd89e2c1 100644 --- a/libraries/wallet/wallet_asset.cpp +++ b/libraries/wallet/wallet_asset.cpp @@ -74,7 +74,7 @@ namespace graphene { namespace wallet { namespace detail { return *opt; } - asset_id_type wallet_api_impl::get_asset_id(string asset_symbol_or_id) const + asset_id_type wallet_api_impl::get_asset_id(const string& asset_symbol_or_id) const { FC_ASSERT( asset_symbol_or_id.size() > 0 ); vector> opt_asset; diff --git a/libraries/wallet/wallet_results.cpp b/libraries/wallet/wallet_results.cpp index b8116195d0..3f881d4138 100644 --- a/libraries/wallet/wallet_results.cpp +++ b/libraries/wallet/wallet_results.cpp @@ -66,12 +66,8 @@ std::map> wallet_a m["get_account_history_by_operations"] = [this](variant result, const fc::variants&) { auto r = result.as( GRAPHENE_MAX_NESTED_OBJECTS ); std::stringstream ss; - ss << "total_count : "; - ss << r.total_count; - ss << " \n"; - ss << "result_count : "; - ss << r.result_count; - ss << " \n"; + ss << "total_count : " << r.total_count << " \n"; + ss << "result_count : " << r.result_count << " \n"; for (operation_detail_ex& d : r.details) { operation_history_object& i = d.op; auto b = _remote_db->get_block_header(i.block_num);