fixup! MurmurHash : Added string constructor and fromString function.

Author: proberts-cinesite

Changes

@@ -9,7 +9,7 @@ Build
 Improvements
 ------------
 
-- Added string constructor and `fromString` function to `IECore.MurmurHash`.
+- Added string constructor and static `fromString` function to `IECore.MurmurHash`.
 
 10.4.2.1 (relative to 10.4.2.0)
 ========

include/IECore/MurmurHash.h

@@ -62,7 +62,7 @@ class IECORE_API MurmurHash
 		inline MurmurHash( const MurmurHash &other );
 
 		// Construct directly from string representation
-		inline explicit MurmurHash( const std::string &repr );
+		explicit MurmurHash( const std::string &repr );
 
 		// Construct directly from known internal values
 		inline MurmurHash( uint64_t h1, uint64_t h2 );
@@ -87,7 +87,7 @@ class IECORE_API MurmurHash
 		inline bool operator < ( const MurmurHash &other ) const;
 
 		std::string toString() const;
-		void fromString( const std::string &repr );
+		static MurmurHash fromString( const std::string &repr );
 
 		// Access internal storage for special cases
 		inline uint64_t h1() const;

include/IECore/MurmurHash.inl

@@ -79,12 +79,6 @@ inline MurmurHash::MurmurHash( const MurmurHash &other )
 {
 }
 
-inline MurmurHash::MurmurHash( const std::string &repr )
-	:	m_h1( 0 ), m_h2( 0 )
-{
-	fromString( repr );
-}
-
 inline MurmurHash::MurmurHash( uint64_t h1, uint64_t h2 )
     :   m_h1( h1 ), m_h2( h2 )
 {

src/IECore/MurmurHash.cpp

@@ -33,27 +33,61 @@
 //////////////////////////////////////////////////////////////////////////
 
 #include "IECore/MurmurHash.h"
+#include "IECore/Exception.h"
+
+#include <boost/format.hpp>
 
 #include <iomanip>
 #include <sstream>
 
 using namespace IECore;
 
-std::string MurmurHash::toString() const
+namespace
+{
+
+std::string internalToString( uint64_t const h1, uint64_t const h2 )
 {
 	std::stringstream s;
-	s << std::hex << std::setfill( '0' ) << std::setw( 16 ) << m_h1 << std::setw( 16 ) << m_h2;
+	s << std::hex << std::setfill( '0' ) << std::setw( 16 ) << h1 << std::setw( 16 ) << h2;
 	return s.str();
 }
 
-void MurmurHash::fromString( const std::string &repr )
+void internalFromString( const std::string &repr, uint64_t &h1, uint64_t &h2 )
 {
+	if( repr.length() != static_cast<std::string::size_type>( 32 ) )
+	{
+		throw Exception(
+			boost::str(
+				boost::format(
+					"Invalid IECore::MurmurHash string representation \"%s\", must have 32 characters" )
+				% repr
+		) );
+	}
+
 	std::stringstream s;
 	s.str( repr.substr( 0, 16 ) );
-	s >> std::hex >> m_h1;
+	s >> std::hex >> h1;
 	s.clear();
 	s.str( repr.substr( 16, 16 ) );
-	s >> std::hex >> m_h2;
+	s >> std::hex >> h2;
+}
+
+} // namespace
+
+MurmurHash::MurmurHash( const std::string &repr )
+	:	m_h1( 0 ), m_h2( 0 )
+{
+	internalFromString( repr, m_h1, m_h2 );
+}
+
+std::string MurmurHash::toString() const
+{
+	return internalToString( m_h1, m_h2 );
+}
+
+MurmurHash MurmurHash::fromString( const std::string &repr )
+{
+	return MurmurHash( repr );
 }
 
 std::ostream &IECore::operator << ( std::ostream &o, const MurmurHash &hash )

src/IECorePython/MurmurHashBinding.cpp

@@ -199,7 +199,8 @@ void IECorePython::bindMurmurHash()
 		.def( "__str__", &MurmurHash::toString )
 		.def( "__hash__", &hash )
 		.def( "toString", &MurmurHash::toString )
-		.def( "fromString", &MurmurHash::fromString )
+		.def( "fromString", (MurmurHash (*)( const std::string & ))&MurmurHash::fromString )
+		.staticmethod( "fromString" )
 		.def( "h1", &MurmurHash::h1 )
 		.def( "h2", &MurmurHash::h2 )
 	;

test/IECore/MurmurHashTest.py

@@ -34,6 +34,7 @@
 
 import unittest
 import imath
+import six
 
 import IECore
 
@@ -71,10 +72,12 @@ def testFromString( self ) :
 		h.append( "hello" )
 
 		s = h.toString()
-		hs = IECore.MurmurHash()
-		hs.fromString( s )
+		hs = IECore.MurmurHash.fromString( s )
 		self.assertEqual( h, hs )
 
+		with six.assertRaisesRegex( self, Exception, ".*must have 32 characters.*" ) :
+			IECore.MurmurHash.fromString( "InvalidStringRepresentation" )
+
 	def testAppend( self ) :
 
 		h = IECore.MurmurHash()