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

Fix code smells

libraries/app/application.cpp

@@ -975,7 +975,7 @@ uint8_t application_impl::get_current_block_interval_in_seconds() const
 namespace graphene { namespace app {
 
 application::application()
-   : my(new detail::application_impl(this))
+   : my(std::make_unique<detail::application_impl>(this))
 {}
 
 application::~application()

libraries/app/database_api.cpp

@@ -48,7 +48,7 @@ namespace graphene { namespace app {
 //////////////////////////////////////////////////////////////////////
 
 database_api::database_api( graphene::chain::database& db, const application_options* app_options )
-   : my( new database_api_impl( db, app_options ) ) {}
+   : my( std::make_unique<database_api_impl>( db, app_options ) ) {}
 
 database_api::~database_api() {}
 

libraries/chain/include/graphene/chain/database.hpp

@@ -306,8 +306,8 @@ namespace graphene { namespace chain {
          template<typename EvaluatorType>
          void register_evaluator()
          {
-            _operation_evaluators[
-               operation::tag<typename EvaluatorType::operation_type>::value].reset( new op_evaluator_impl<EvaluatorType>() );
+            _operation_evaluators[operation::tag<typename EvaluatorType::operation_type>::value]
+                  = std::make_unique<op_evaluator_impl<EvaluatorType>>();
          }
 
          //////////////////// db_balance.cpp ////////////////////

libraries/db/include/graphene/db/index.hpp

@@ -169,7 +169,7 @@ namespace graphene { namespace db {
          template<typename T, typename... Args>
          T* add_secondary_index(Args... args)
          {
-            _sindex.emplace_back( new T(args...) );
+            _sindex.emplace_back( std::make_unique<T>(args...) );
             return static_cast<T*>(_sindex.back().get());
          }
 

libraries/db/include/graphene/db/object.hpp

@@ -88,7 +88,7 @@ namespace graphene { namespace db {
       public:
          virtual unique_ptr<object> clone()const
          {
-            return unique_ptr<object>(new DerivedClass( *static_cast<const DerivedClass*>(this) ));
+            return unique_ptr<object>( std::make_unique<DerivedClass>( *static_cast<const DerivedClass*>(this) ) );
          }
 
          virtual void    move_from( object& obj )

libraries/db/include/graphene/db/object_database.hpp

@@ -138,7 +138,7 @@ namespace graphene { namespace db {
             if( _index[ObjectType::space_id].size() <= ObjectType::type_id  )
                 _index[ObjectType::space_id].resize( 255 );
             assert(!_index[ObjectType::space_id][ObjectType::type_id]);
-            unique_ptr<index> indexptr( new IndexType(*this) );
+            unique_ptr<index> indexptr( std::make_unique<IndexType>(*this) );
             _index[ObjectType::space_id][ObjectType::type_id] = std::move(indexptr);
             return static_cast<IndexType*>(_index[ObjectType::space_id][ObjectType::type_id].get());
          }

libraries/db/include/graphene/db/simple_index.hpp

@@ -45,7 +45,7 @@ namespace graphene { namespace db {
              auto id = get_next_id();
              auto instance = id.instance();
              if( instance >= _objects.size() ) _objects.resize( instance + 1 );
-             _objects[instance].reset(new T);
+             _objects[instance] = std::make_unique<T>();
              _objects[instance]->id = id;
              constructor( *_objects[instance] );
              _objects[instance]->id = id; // just in case it changed
@@ -65,7 +65,7 @@ namespace graphene { namespace db {
             assert( nullptr != dynamic_cast<T*>(&obj) );
             if( _objects.size() <= instance ) _objects.resize( instance+1 );
             assert( !_objects[instance] );
-            _objects[instance].reset( new T( std::move( static_cast<T&>(obj) ) ) );
+            _objects[instance] = std::make_unique<T>( std::move( static_cast<T&>(obj) ) );
             return *_objects[instance];
          }
 

libraries/net/include/graphene/net/peer_connection.hpp

@@ -69,8 +69,7 @@ namespace graphene { namespace net
       virtual message get_message_for_item(const item_id& item) = 0;
     };
 
-    class peer_connection;
-    typedef std::shared_ptr<peer_connection> peer_connection_ptr;
+    using peer_connection_ptr = std::shared_ptr<peer_connection>;
     class peer_connection : public message_oriented_connection_delegate,
                             public std::enable_shared_from_this<peer_connection>
     {
@@ -117,7 +116,7 @@ namespace graphene { namespace net
         fc::time_point transmission_start_time;
         fc::time_point transmission_finish_time;
 
-        queued_message(fc::time_point enqueue_time = fc::time_point::now()) :
+        explicit queued_message(fc::time_point enqueue_time = fc::time_point::now()) :
           enqueue_time(enqueue_time)
         {}
 
@@ -126,7 +125,7 @@ namespace graphene { namespace net
          * it is sitting on the queue
          */
         virtual size_t get_size_in_queue() = 0;
-        virtual ~queued_message() {}
+        virtual ~queued_message() = default;
       };
 
       /* when you queue up a 'real_queued_message', a full copy of the message is
@@ -155,8 +154,8 @@ namespace graphene { namespace net
       {
         item_id item_to_send;
 
-        virtual_queued_message(item_id item_to_send) :
-          item_to_send(std::move(item_to_send))
+        explicit virtual_queued_message(item_id the_item_to_send) :
+          item_to_send(std::move(the_item_to_send))
         {}
 
         message get_message(peer_connection_delegate* node) override;

libraries/net/include/graphene/net/peer_database.hpp

@@ -79,7 +79,7 @@ namespace graphene { namespace net {
     public:
       peer_database_iterator();
       ~peer_database_iterator();
-      explicit peer_database_iterator(peer_database_iterator_impl* impl);
+      explicit peer_database_iterator( std::unique_ptr<peer_database_iterator_impl>&& impl );
       peer_database_iterator( const peer_database_iterator& c );
 
     private:
@@ -97,7 +97,7 @@ namespace graphene { namespace net {
   {
   public:
     peer_database();
-    ~peer_database();
+    virtual ~peer_database();
 
     void open(const fc::path& databaseFilename);
     void close();
@@ -109,7 +109,7 @@ namespace graphene { namespace net {
     potential_peer_record lookup_or_create_entry_for_endpoint(const fc::ip::endpoint& endpointToLookup);
     fc::optional<potential_peer_record> lookup_entry_for_endpoint(const fc::ip::endpoint& endpointToLookup);
 
-    typedef detail::peer_database_iterator iterator;
+    using iterator = detail::peer_database_iterator;
     iterator begin() const;
     iterator end() const;
     size_t size() const;

libraries/net/message_oriented_connection.cpp

@@ -273,17 +273,18 @@ namespace graphene { namespace net {
            elog("Trying to send a message larger than MAX_MESSAGE_SIZE. This probably won't work...");
         //pad the message we send to a multiple of 16 bytes
         size_t size_with_padding = 16 * ((size_of_message_and_header + 15) / 16);
-        std::unique_ptr<char[]> padded_message(new char[size_with_padding]);
+        std::vector<char> padded_message( size_with_padding );
 
-        memcpy(padded_message.get(), (char*)&message_to_send, sizeof(message_header));
-        memcpy(padded_message.get() + sizeof(message_header), message_to_send.data.data(), message_to_send.size.value() );
-        char* padding_space = padded_message.get() + sizeof(message_header) + message_to_send.size.value();
+        memcpy( padded_message.data(), (const char*)&message_to_send, sizeof(message_header) );
+        memcpy( padded_message.data() + sizeof(message_header), message_to_send.data.data(),
+                message_to_send.size.value() );
+        char* padding_space = padded_message.data() + sizeof(message_header) + message_to_send.size.value();
         memset(padding_space, 0, size_with_padding - size_of_message_and_header);
-        _sock.write(padded_message.get(), size_with_padding);
+        _sock.write( padded_message.data(), size_with_padding );
         _sock.flush();
         _bytes_sent += size_with_padding;
         _last_message_sent_time = fc::time_point::now();
-      } FC_RETHROW_EXCEPTIONS( warn, "unable to send message" );
+      } FC_RETHROW_EXCEPTIONS( warn, "unable to send message" )
     }
 
     void message_oriented_connection_impl::close_connection()
@@ -355,7 +356,7 @@ namespace graphene { namespace net {
 
 
   message_oriented_connection::message_oriented_connection(message_oriented_connection_delegate* delegate) :
-    my(new detail::message_oriented_connection_impl(this, delegate))
+    my( std::make_unique<detail::message_oriented_connection_impl>(this, delegate) )
   {
   }
 

libraries/net/peer_connection.cpp

@@ -107,8 +107,14 @@ namespace graphene { namespace net
       // current task yields.  In the (not uncommon) case where it is the task executing
       // connect_to or read_loop, this allows the task to finish before the destructor is forced
       // to cancel it.
-      return peer_connection_ptr(new peer_connection(delegate));
-      //, [](peer_connection* peer_to_delete){ fc::async([peer_to_delete](){delete peer_to_delete;}); });
+
+      // Implementation: derive peer_connection so that make_shared has access to the constructor
+      class peer_connection_subclass : public peer_connection
+      {
+      public:
+         explicit peer_connection_subclass(peer_connection_delegate* delegate) : peer_connection(delegate) {}
+      };
+      return std::make_shared<peer_connection_subclass>(delegate);
     }
 
     void peer_connection::destroy()
@@ -374,17 +380,18 @@ namespace graphene { namespace net
     {
       VERIFY_CORRECT_THREAD();
       //dlog("peer_connection::send_message() enqueueing message of type ${type} for peer ${endpoint}",
-      //     ("type", message_to_send.msg_type)("endpoint", get_remote_endpoint()));
-      std::unique_ptr<queued_message> message_to_enqueue(new real_queued_message(message_to_send, message_send_time_field_offset));
+      //     ("type", message_to_send.msg_type)("endpoint", get_remote_endpoint())); // for debug
+      auto message_to_enqueue = std::make_unique<real_queued_message>(
+                                      message_to_send, message_send_time_field_offset );
       send_queueable_message(std::move(message_to_enqueue));
     }
 
     void peer_connection::send_item(const item_id& item_to_send)
     {
       VERIFY_CORRECT_THREAD();
       //dlog("peer_connection::send_item() enqueueing message of type ${type} for peer ${endpoint}",
-      //     ("type", item_to_send.item_type)("endpoint", get_remote_endpoint()));
-      std::unique_ptr<queued_message> message_to_enqueue(new virtual_queued_message(item_to_send));
+      //     ("type", item_to_send.item_type)("endpoint", get_remote_endpoint())); // for debug
+      auto message_to_enqueue = std::make_unique<virtual_queued_message>(item_to_send);
       send_queueable_message(std::move(message_to_enqueue));
     }
 

libraries/net/peer_database.cpp

@@ -173,12 +173,14 @@ namespace graphene { namespace net {
 
     peer_database::iterator peer_database_impl::begin() const
     {
-      return peer_database::iterator(new peer_database_iterator_impl(_potential_peer_set.get<last_seen_time_index>().begin()));
+      return peer_database::iterator( std::make_unique<peer_database_iterator_impl>(
+                   _potential_peer_set.get<last_seen_time_index>().begin() ) );
     }
 
     peer_database::iterator peer_database_impl::end() const
     {
-      return peer_database::iterator(new peer_database_iterator_impl(_potential_peer_set.get<last_seen_time_index>().end()));
+      return peer_database::iterator( std::make_unique<peer_database_iterator_impl>(
+                   _potential_peer_set.get<last_seen_time_index>().end() ) );
     }
 
     size_t peer_database_impl::size() const
@@ -194,8 +196,8 @@ namespace graphene { namespace net {
     {
     }
 
-    peer_database_iterator::peer_database_iterator(peer_database_iterator_impl* impl) :
-      my(impl)
+    peer_database_iterator::peer_database_iterator( std::unique_ptr<peer_database_iterator_impl>&& impl) :
+      my( std::move(impl) )
     {
     }
 
@@ -217,7 +219,7 @@ namespace graphene { namespace net {
   } // end namespace detail
 
   peer_database::peer_database() :
-    my(new detail::peer_database_impl)
+    my( std::make_unique<detail::peer_database_impl>() )
   {
   }
 

libraries/plugins/account_history/account_history_plugin.cpp

@@ -283,7 +283,7 @@ void account_history_plugin_impl::add_account_history( const account_id_type acc
 
 
 account_history_plugin::account_history_plugin() :
-   my( new detail::account_history_plugin_impl(*this) )
+   my( std::make_unique<detail::account_history_plugin_impl>(*this) )
 {
 }
 

libraries/plugins/api_helper_indexes/api_helper_indexes.cpp

@@ -151,7 +151,7 @@ class api_helper_indexes_impl
 } // end namespace detail
 
 api_helper_indexes::api_helper_indexes() :
-   my( new detail::api_helper_indexes_impl(*this) )
+   my( std::make_unique<detail::api_helper_indexes_impl>(*this) )
 {
 }
 

libraries/plugins/custom_operations/custom_operations_plugin.cpp

@@ -106,7 +106,7 @@ custom_operations_plugin_impl::~custom_operations_plugin_impl()
 } // end namespace detail
 
 custom_operations_plugin::custom_operations_plugin() :
-   my( new detail::custom_operations_plugin_impl(*this) )
+   my( std::make_unique<detail::custom_operations_plugin_impl>(*this) )
 {
 }
 

libraries/plugins/delayed_node/delayed_node_plugin.cpp

@@ -89,7 +89,7 @@ void delayed_node_plugin::connect()
 void delayed_node_plugin::plugin_initialize(const boost::program_options::variables_map& options)
 {
    FC_ASSERT(options.count("trusted-node") > 0);
-   my = std::unique_ptr<detail::delayed_node_plugin_impl>{ new detail::delayed_node_plugin_impl() };
+   my = std::make_unique<detail::delayed_node_plugin_impl>();
    my->remote_endpoint = "ws://" + options.at("trusted-node").as<std::string>();
 }
 

libraries/plugins/elasticsearch/elasticsearch_plugin.cpp

@@ -434,7 +434,7 @@ void elasticsearch_plugin_impl::populateESstruct()
 } // end namespace detail
 
 elasticsearch_plugin::elasticsearch_plugin() :
-   my( new detail::elasticsearch_plugin_impl(*this) )
+   my( std::make_unique<detail::elasticsearch_plugin_impl>(*this) )
 {
 }
 

libraries/plugins/es_objects/es_objects.cpp

@@ -275,7 +275,7 @@ es_objects_plugin_impl::~es_objects_plugin_impl()
 } // end namespace detail
 
 es_objects_plugin::es_objects_plugin() :
-   my( new detail::es_objects_plugin_impl(*this) )
+   my( std::make_unique<detail::es_objects_plugin_impl>(*this) )
 {
 }
 

libraries/plugins/grouped_orders/grouped_orders_plugin.cpp

@@ -242,7 +242,7 @@ grouped_orders_plugin_impl::~grouped_orders_plugin_impl()
 
 
 grouped_orders_plugin::grouped_orders_plugin() :
-   my( new detail::grouped_orders_plugin_impl(*this) )
+   my( std::make_unique<detail::grouped_orders_plugin_impl>(*this) )
 {
 }
 

libraries/plugins/market_history/market_history_plugin.cpp

@@ -736,7 +736,7 @@ void market_history_plugin_impl::update_liquidity_pool_histories(
 
 
 market_history_plugin::market_history_plugin() :
-   my( new detail::market_history_plugin_impl(*this) )
+   my( std::make_unique<detail::market_history_plugin_impl>(*this) )
 {
 }
 

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, uint32_t limit)const;
+      vector<operation_detail>  get_account_history(const string& account_name_or_id, uint32_t limit)const;
 
       /** Returns the relative operations on the named account from start number.
        *
@@ -136,7 +136,7 @@ class wallet_api
        * @param start  the sequence number where to start looping back throw the history
        * @returns a list of \c operation_history_objects
        */
-     vector<operation_detail>  get_relative_account_history( string account_name_or_id, uint32_t stop,
+     vector<operation_detail>  get_relative_account_history( const string& account_name_or_id, uint32_t stop,
                                                              uint32_t limit, uint32_t start )const;
 
       /**
@@ -241,8 +241,8 @@ class wallet_api
        * @param limit the max number of entries to return (from start number)
        * @returns account_history_operation_detail
        */
-      account_history_operation_detail get_account_history_by_operations( string account_name_or_id,
-                                                                          flat_set<uint16_t> operation_types,
+      account_history_operation_detail get_account_history_by_operations( const string& account_name_or_id,
+                                                                          const flat_set<uint16_t>& operation_types,
                                                                           uint32_t start, uint32_t limit);
 
       /** Returns the block chain's rapidly-changing properties.

libraries/wallet/wallet.cpp

@@ -151,7 +151,7 @@ namespace graphene { namespace wallet {
 namespace graphene { namespace wallet {
 
 wallet_api::wallet_api(const wallet_data& initial_data, fc::api<login_api> rapi)
-   : my(new detail::wallet_api_impl(*this, initial_data, rapi))
+   : my( std::make_unique<detail::wallet_api_impl>(*this, initial_data, rapi) )
 {
 }
 
@@ -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<operation_detail> wallet_api::get_account_history(string name, uint32_t limit)const
+vector<operation_detail> wallet_api::get_account_history(const string& name, uint32_t limit)const
 {
    vector<operation_detail> result;
 
@@ -335,7 +335,7 @@ vector<operation_detail> wallet_api::get_account_history(string name, uint32_t l
 }
 
 vector<operation_detail> wallet_api::get_relative_account_history(
-      string name,
+      const string& name,
       uint32_t stop,
       uint32_t limit,
       uint32_t start)const
@@ -375,15 +375,14 @@ vector<operation_detail> wallet_api::get_relative_account_history(
 }
 
 account_history_operation_detail wallet_api::get_account_history_by_operations(
-      string name,
-      flat_set<uint16_t> operation_types,
+      const string& name,
+      const flat_set<uint16_t>& operation_types,
       uint32_t start,
       uint32_t limit)
 {
     account_history_operation_detail result;
-    auto account_id = get_account(name).get_id();
 
-    const auto& account = my->get_account(account_id);
+    const auto& account = my->get_account(name);
     const auto& stats = my->get_object(account.statistics);
 
     // sequence of account_transaction_history_object start with 1