Issues #276, #1163 - introduce class IncomingParam

src/plugin/COM_Value.h

 /**
  * \file COM_Value.h Support for the values used in COM and Automation.
  */
 #ifndef COM_VALUE_H
 #define COM_VALUE_H
 
 #include <wtypes.h>
 #include <string>
 
 namespace AdblockPlus
@@ skipping 11 matching lines @@
      * In both cases, we must release an allocated string, whether or not we allocated it.
      * Because of this commonality, it's possible to combine these two uses into a single class.
      *
      * The two life cycles differ in how the allocation is done.
      * When used as an argument, the caller must allocate.
      * In this case the caller uses the non-trivial constructor, which allocates a copy of its argument.
      * When used for a result, the called function allocates.
      * In this case the caller uses the default constructor and the address-of operator;
      *   the address-of operator does not allocate and its pointer must be assigned an allocated value.
      *
-     * Copy/move constructor/assignment are all deleted, not because they couldn't be implemented, 
+     * Copy/move constructor/assignment are all deleted, not because they couldn't be implemented,
      *   but because these class are meant to be used in tight conjunction with API calls.
      * Thus their design use requires that they not leave this proximity.
      *
      * \par Reference
      *   MSDN "Allocating and Releasing Memory for a BSTR" http://msdn.microsoft.com/en-us/library/vstudio/xda6xzx7%28v=vs.120%29.aspx
      *   "When you call into a function that expects a BSTR argument, you must allocate the memory for the BSTR before the call and release it afterwards."
      *   "When you call into a function that returns a BSTR, you must free the string yourself."
      *
      * \invariant Either bstr == nullptr or bstr is a non-null system-allocated BSTR.
      */
     class BSTR_Argument
     {
       /**
        * The underlying BSTR pointer.
        */
-       BSTR bstr;
+      BSTR bstr;
 
     public:
       /**
        * Default constructor has the value of the empty string.
        */
       BSTR_Argument()
-        : bstr( nullptr )
+        : bstr(nullptr)
       {
       }
 
       /**
        * Constructor from std::wstring.
        */
-      BSTR_Argument( const std::wstring& s );
+      BSTR_Argument(const std::wstring& s);
 
       /**
        * Destructor
        *
        * The destructor frees the BSTR.
        * Do not use this class when freeing the BSTR is not our responsibility.
        */
       ~BSTR_Argument();
 
       /**
        * Conversion operator to BSTR.
        */
       operator BSTR() const
       {
         return bstr;
       }
 
       /**
        * Conversion operator to std::wstring
        */
       operator std::wstring() const;
 
       /**
        * Address-of operator.
        *
        * This operator is used for assignment directly into our underlying pointer.
        * In order to avoid leaking memory, this operator also implicitly assigns the null string.
        * Specifically, this operator is not 'const'.
        *
-       * \postcondition
+       * \par postcondition
        *   bstr == nullptr
        */
       BSTR* operator&();
 
     private:
       /**
        * Copy constructor is deleted
        */
-      BSTR_Argument( const BSTR_Argument& ); // = delete
+      BSTR_Argument(const BSTR_Argument&);   // = delete
 
       /**
        * Move constructor is deleted
        */
-      BSTR_Argument( BSTR_Argument&& ); // = delete
+      BSTR_Argument(BSTR_Argument&&);   // = delete
 
       /**
        * Copy assignment is deleted
        */
-      BSTR_Argument& operator=( const BSTR_Argument& ); // = delete
+      BSTR_Argument& operator=(const BSTR_Argument&);   // = delete
 
       /**
        * Move assignment is deleted
        */
-      BSTR_Argument& operator=( BSTR_Argument&& ); // = delete
+      BSTR_Argument& operator=(BSTR_Argument&&);   // = delete
     };
 
     /**
-     * Constructor class for BSTR [in] parameters arriving from our COM entry points.
+     * Constructor class for [in] parameters arriving from our COM entry points.
      *
-     * The life cycle of a BSTR value that comes to us as an [in] parameter is entirely managed by the caller.
-     * Thus this class simply derives from std::wstring and provides an appropriate constructor.
+     * The life cycle of a VARIANT value that comes to us as an [in] parameter is entirely managed by the caller.
+     * Thus the responsbilities of this class are for type checking and value conversion,
+     *   but not life cycle mangement.
      *
-     * This class is a narrow technique toward a larger goal of eliminating CComBSTR,
-     *   which had been used for these parameters.
-     * All the uses, however, had been for IDispatch entry points, which are all VARIANT.
-     * In all cases, the caller much first check the type of the VARIANT structure before constructing an instance.
-     * Perhaps better would be a class, say, wstring_Incoming_Param that took a VARIANT as a constructor argument
-     *   and converted it to a string.
-     * Such a class, however, is beyond the scope of the work that this class was a part of.
+     * This class is specifically for use within COM server functions such as for IDispatch::Invoke.
+     *
+     * \par Status
+     *   This class is, in part, written for issue #1163 "Clean up IDispatch::Invoke implementations".
+     *   
      */
-    class Incoming_Param
-      : public std::wstring 
+    class IncomingParam
     {
+      /**
+       * The underlying variant value.
+       *
+       * It's stored as received and does receive any life cycle treatment.
+       */
+      VARIANT var;
+
     public:
-      Incoming_Param( BSTR b );

[sergei, 2014/07/28 09:24:31]

Instead of commenting all bad things here, I will say only the desired result.
Replace this class by the method
`std::wstring our-namespace::to_wstring(const BSTR value)`.

[Eric, 2014/07/29 19:53:08]

Oh, that's just awful. If there's one thing I've learned about dealing with COM
interface types, it's that conversion functions that ignore their lifecycle
issues are a Very Bad Idea. Using a generic name for a conversion function is
asking for trouble.

Whatever. This was supposed to be a temporary class anyway, as the comment very
clearly indicated. I've just rewritten the whole thing as I was going to get to
later.

+      IncomingParam( VARIANT v )
+        : var( v )
+      {
+      }
+
+      /**
+       * Predicate whether the variant is convertible to std::wstring.
+       *
+       * This predicate incorporates both type compatibility and implementation support.
+       * Some types that could be converted are not, simply because they're not used in the present code base.
+       */
+      bool wstringConvertible() const;
+
+      /**
+       * Convert variant to std::wstring and if not convertible return the empty string.
+       */
+      std::wstring wstringValueNoexcept() const;
     };
   }
 }
 
 #endif