Merge pull request #2390 from bitshares/fix-code-smells

Fix code smells

libraries/app/api.cpp

@@ -340,7 +340,7 @@ namespace graphene { namespace app {
 
     vector<operation_history_object> 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<operation_history_object> 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<operation_history_object> 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<uint16_t> 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,

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<graphene::api_helper_indexes::asset_in_liquidity_pools_index>();
    }
-   catch( fc::assert_exception& e )
+   catch( const fc::assert_exception& )
    {
       asset_in_liquidity_pools_index = nullptr;
    }
@@ -1827,10 +1827,10 @@ vector<extended_liquidity_pool_object> database_api_impl::get_liquidity_pools_by
 }
 
 vector<extended_liquidity_pool_object> database_api::get_liquidity_pools_by_one_asset(
-            std::string asset_symbol_or_id,
-            optional<uint32_t> limit,
-            optional<liquidity_pool_id_type> start_id,
-            optional<bool> with_statistics )const
+            const std::string& asset_symbol_or_id,
+            const optional<uint32_t>& limit,
+            const optional<liquidity_pool_id_type>& start_id,
+            const optional<bool>& with_statistics )const
 {
    return my->get_liquidity_pools_by_one_asset(
             asset_symbol_or_id,
@@ -1840,10 +1840,10 @@ vector<extended_liquidity_pool_object> database_api::get_liquidity_pools_by_one_
 }
 
 vector<extended_liquidity_pool_object> database_api_impl::get_liquidity_pools_by_one_asset(
-            std::string asset_symbol_or_id,
-            optional<uint32_t> olimit,
-            optional<liquidity_pool_id_type> ostart_id,
-            optional<bool> with_statistics )const
+            const std::string& asset_symbol_or_id,
+            const optional<uint32_t>& olimit,
+            const optional<liquidity_pool_id_type>& ostart_id,
+            const optional<bool>& 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,9 +1869,10 @@ vector<extended_liquidity_pool_object> database_api_impl::get_liquidity_pools_by
    vector<extended_liquidity_pool_object> results;
 
    results.reserve( limit );
-   for ( ; itr != pools.end() && results.size() < limit; ++itr )
+   while( itr != pools.end() && results.size() < limit )
    {
       results.emplace_back( extend_liquidity_pool( (*itr)(_db), with_stats ) );
+      ++itr;
    }
 
    return results;

libraries/app/database_api_impl.hxx

@@ -154,10 +154,10 @@ class database_api_impl : public std::enable_shared_from_this<database_api_impl>
             optional<liquidity_pool_id_type> start_id = optional<liquidity_pool_id_type>(),
             optional<bool> with_statistics = false )const;
       vector<extended_liquidity_pool_object> get_liquidity_pools_by_one_asset(
-            std::string asset_symbol_or_id,
-            optional<uint32_t> limit = 101,
-            optional<liquidity_pool_id_type> start_id = optional<liquidity_pool_id_type>(),
-            optional<bool> with_statistics = false )const;
+            const std::string& asset_symbol_or_id,
+            const optional<uint32_t>& limit = 101,
+            const optional<liquidity_pool_id_type>& start_id = optional<liquidity_pool_id_type>(),
+            const optional<bool>& with_statistics = false )const;
       vector<extended_liquidity_pool_object> get_liquidity_pools_by_both_assets(
             std::string asset_symbol_or_id_a,
             std::string asset_symbol_or_id_b,

libraries/app/include/graphene/app/api.hpp

@@ -136,7 +136,7 @@ namespace graphene { namespace app {
          vector<operation_history_object> 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<uint16_t> operation_types,
             uint32_t start,
-            unsigned limit
+            uint32_t limit
          )const;
 
          /**
@@ -168,10 +168,10 @@ namespace graphene { namespace app {
           */
          vector<operation_history_object> 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<operation_history_object> 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;
 
          /**

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<extended_liquidity_pool_object> get_liquidity_pools_by_one_asset(
-            std::string asset_symbol_or_id,
-            optional<uint32_t> limit = 101,
-            optional<liquidity_pool_id_type> start_id = optional<liquidity_pool_id_type>(),
-            optional<bool> with_statistics = false )const;
+            const std::string& asset_symbol_or_id,
+            const optional<uint32_t>& limit = 101,
+            const optional<liquidity_pool_id_type>& start_id = optional<liquidity_pool_id_type>(),
+            const optional<bool>& with_statistics = false )const;
 
       /**
        * @brief Get a list of liquidity pools by the symbols or IDs of the two assets in the pool

libraries/chain/include/graphene/chain/config.hpp

@@ -25,12 +25,14 @@
 
 #include <graphene/protocol/config.hpp>
 
+#include <string>
+
 #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

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<const liquidity_pool_object&>( objct );
+   const auto& o = static_cast<const liquidity_pool_object&>( 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<const liquidity_pool_object&>( objct );
+   const auto& o = static_cast<const liquidity_pool_object&>( 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<liquidity_pool_id_type>& asset_in_liquidity_pools_index::get_liquidity_pools_by_asset(

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<liquidity_pool_id_type>& 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<detail::api_helper_indexes_impl> my;
 
    private:
+      std::unique_ptr<detail::api_helper_indexes_impl> my;
       amount_in_collateral_index* amount_in_collateral_idx = nullptr;
       asset_in_liquidity_pools_index* asset_in_liquidity_pools_idx = nullptr;
 };

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<operation_detail>  get_account_history(string account_name_or_id, int limit)const;
+      vector<operation_detail>  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<operation_detail>  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<uint16_t> 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,30 +291,30 @@ 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; }
 
       /**
        * Lookup the id of an asset.
        * @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; }
 
       /**
        * Lookup the symbol of an asset. Synonym of @ref get_asset_symbol.
        * @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<transaction_id_type,signed_transaction> transfer2(string from,
-                                                             string to,
-                                                             string amount,
-                                                             string asset_symbol_or_id,
-                                                             string memo ) {
+      pair<transaction_id_type,signed_transaction> 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);
       }