Moved all pipe functionality into a self-containing Communication::Pipe class

src/shared/Communication.h

Index: src/shared/Communication.h
===================================================================
--- a/src/shared/Communication.h
+++ b/src/shared/Communication.h
@@ -1,12 +1,14 @@
 #ifndef COMMUNICATION_H
 #define COMMUNICATION_H
 
+#include <memory>
 #include <sstream>
+#include <stdexcept>
 #include <stdint.h>
 #include <string>
 #include <vector>
 #include <Windows.h>
 
 namespace Communication
 {
   extern const std::wstring pipeName;

[Felix Dahlke, 2013/06/03 12:18:47]

Since we're really always using the same pipeName, I guess it makes sense to
have it as a private constant and not passing a name in. I don't see a need for
having more than one pipe.

[Wladimir Palant, 2013/06/03 14:00:26]

The tests use a different pipe name.

@@ -20,45 +22,55 @@ namespace Communication
     InputBuffer(const std::string& data) : buffer(data) {}
     InputBuffer& operator>>(std::string& value) { return ReadString(value, TYPE_STRING); }
     InputBuffer& operator>>(std::wstring& value) { return ReadString(value, TYPE_WSTRING); }
     InputBuffer& operator>>(int64_t& value) { return Read(value, TYPE_INT64); }
     InputBuffer& operator>>(int32_t& value) { return Read(value, TYPE_INT32); }
     InputBuffer& operator>>(bool& value) { return Read(value, TYPE_BOOL); }
   private:
     std::istringstream buffer;
+    int32_t currentType;

[Felix Dahlke, 2013/06/03 12:18:47]

I'd like to have the typedef here I asked for in the other review.

[Wladimir Palant, 2013/06/03 14:00:26]

You are getting it in the other review :)

+    bool hasType;
+
+    void CheckType(ValueType expectedType)

[Felix Dahlke, 2013/06/03 12:18:47]

I'd prefer to see this function in the cpp, it's a bit big for an inline
function IMO.

+    {
+      if (!hasType)
+        ReadBinary(currentType);
+
+      if (currentType != expectedType)
+      {
+        // Make sure we don't attempt to read the type again
+        hasType = true;
+        throw new std::runtime_error("Unexpected type found in input buffer");
+      }
+      else
+        hasType = false;
+    }
 
     template<class T>
     InputBuffer& ReadString(T& value, ValueType expectedType)
     {
-      int32_t type;
-      ReadBinary(type);
-      if (type != expectedType)
-        throw new std::runtime_error("Unexpected type found in input buffer");
+      CheckType(expectedType);
 
       uint32_t length;
       ReadBinary(length);
 
       std::auto_ptr<T::value_type> data(new T::value_type[length]);
       buffer.read(reinterpret_cast<char*>(data.get()), sizeof(T::value_type) * length);
       if (buffer.fail())
         throw new std::runtime_error("Unexpected end of input buffer");
 
       value.assign(data.get(), length);
       return *this;
     }
 
     template<class T>
     InputBuffer& Read(T& value, ValueType expectedType)
     {
-      int32_t type;
-      ReadBinary(type);
-      if (type != expectedType)
-        throw new std::runtime_error("Unexpected type found in input buffer");
-
+      CheckType(expectedType);
       ReadBinary(value);
       return *this;
     }
 
     template<class T>
     void ReadBinary(T& value)
     {
       buffer.read(reinterpret_cast<char*>(&value), sizeof(T));
@@ -114,13 +126,31 @@ namespace Communication
     void WriteBinary(const T& value)
     {
       buffer.write(reinterpret_cast<const char*>(&value), sizeof(T));
       if (buffer.fail())
         throw new std::runtime_error("Unexpected error writing to output buffer");
     }
   };
 
-  InputBuffer ReadMessage(HANDLE pipe);
-  void WriteMessage(HANDLE pipe, OutputBuffer& message);
+  class PipeConnectionError : public std::runtime_error
+  {
+  public:
+    PipeConnectionError();
+  };
+
+  class Pipe
+  {
+  public:
+    enum Mode {MODE_CREATE, MODE_CONNECT};
+
+    Pipe(const std::wstring& name, Mode mode);
+    ~Pipe();
+
+    InputBuffer ReadMessage();
+    void WriteMessage(OutputBuffer& message);
+
+  protected:
+    HANDLE pipe;
+  };
 }
 
 #endif