More parser clean up - color parser

Dropped color_factory class in favor of single color_parser function. Moved implementation to new color_factory.cpp since it is odd to have two headers (color.hpp, color_factory.hpp) and only one source file.

demo/c++/rundemo.cpp

@@ -61,7 +61,7 @@ int main ( int argc , char** argv)
         freetype_engine::register_font(mapnik_dir + "/fonts/DejaVuSans.ttf");
 
         Map m(800,600);
-        m.set_background(color_factory::from_string("white"));
+        m.set_background(parse_color("white"));
 
         // create styles
 

include/mapnik/color.hpp

@@ -47,37 +47,51 @@ private:
 
 public:
     color()
-        : red_(0xff),
+      : red_(0xff),
         green_(0xff),
         blue_(0xff),
         alpha_(0xff)
         {}
 
     color(unsigned red, unsigned green, unsigned blue, unsigned alpha = 0xff)
-        :  red_(red),
+      : red_(red),
         green_(green),
         blue_(blue),
         alpha_(alpha)
         {}
 
-    color( std::string const& css_string);
-
     color(const color& rhs)
-        : red_(rhs.red_),
+      : red_(rhs.red_),
         green_(rhs.green_),
         blue_(rhs.blue_),
         alpha_(rhs.alpha_)
         {}
 
-    color& operator=(const color& rhs)
+    color( std::string const& str);
-        {
+
-            if (this==&rhs) return *this;
+    std::string to_string() const;
-            red_=rhs.red_;
+    std::string to_hex_string() const;
-            green_=rhs.green_;
+
-            blue_=rhs.blue_;
+    color& operator=(color const& rhs)
-            alpha_=rhs.alpha_;
+    {
+        if (this==&rhs) 
             return *this;
-        }
+        
+        red_   = rhs.red_;
+        green_ = rhs.green_;
+        blue_  = rhs.blue_;
+        alpha_ = rhs.alpha_;
+        
+        return *this;
+    }
+
+    inline bool operator==(color const& rhs) const
+    {
+        return (red_== rhs.red()) &&
+               (green_ == rhs.green()) &&
+               (blue_  == rhs.blue()) &&
+               (alpha_ == rhs.alpha());
+    }
 
     inline unsigned red() const
     {
@@ -123,18 +137,6 @@ public:
         return (alpha_ << 24) | (blue_ << 16) | (green_ << 8) | (red_) ;
 #endif
     }
-
-    inline bool operator==(color const& rhs) const
-    {
-        return (red_== rhs.red()) &&
-            (green_ == rhs.green()) &&
-            (blue_  == rhs.blue()) &&
-            (alpha_ == rhs.alpha());
-
-    }
-
-    std::string to_string() const;
-    std::string to_hex_string() const;
 };
 
 template <typename charT, typename traits>

include/mapnik/color_factory.hpp

@@ -24,27 +24,16 @@
 #define MAPNIK_COLOR_FACTORY_HPP
 
 // mapnik
-#include <mapnik/config.hpp>
+#include <mapnik/color.hpp>
+#include <mapnik/css_color_grammar.hpp>
 
-// boost
+#include <string>
-#include <boost/utility.hpp>
 
 namespace mapnik {
 
-class color;
+MAPNIK_DECL mapnik::color parse_color(std::string const& str);
+MAPNIK_DECL mapnik::color parse_color(std::string const& str, mapnik::css_color_grammar<std::string::const_iterator> const& g);
 
-template <typename Iterator> struct css_color_grammar;
-class MAPNIK_DECL color_factory : boost::noncopyable
-{
-public:
-
-    static void init_from_string(color & c, std::string const& css_color);
-
-    static bool parse_from_string(color & c, std::string const& css_color,
-                                  mapnik::css_color_grammar<std::string::const_iterator> const& g);
-
-    static color from_string(std::string const& css_color);
-};
 }
 
 #endif // MAPNIK_COLOR_FACTORY_HPP

src/build.py

@@ -187,6 +187,7 @@ source = Split(
     text_properties.cpp
     xml_tree.cpp
     config_error.cpp
+    color_factory.cpp
     """
     )
 

src/color.cpp

@@ -27,22 +27,15 @@
 
 // boost
 #include <boost/format.hpp>
-#include <boost/version.hpp>
 
 // stl
 #include <sstream>
 
-#include <mapnik/css_color_grammar.hpp>
-
 namespace mapnik {
 
-color::color( std::string const& css_string)
+color::color(std::string const& str)
-    : red_(0),
-      green_(0),
-      blue_(0),
-      alpha_(0xff)
 {
-    color_factory::init_from_string(*this,css_string);
+    *this = parse_color(str);
 }
 
 std::string color::to_string() const
@@ -85,90 +78,5 @@ std::string color::to_hex_string() const
     }
 }
 
-
-/****************************************************************************/
-void color_factory::init_from_string(color & c, std::string const& css_color)
-{
-    typedef std::string::const_iterator iterator_type;
-    typedef mapnik::css_color_grammar<iterator_type> css_color_grammar;
-
-    css_color_grammar g;
-    iterator_type first = css_color.begin();
-    iterator_type last =  css_color.end();
-    // boost 1.41 -> 1.44 compatibility, to be removed in mapnik 2.1 (dane)
-#if BOOST_VERSION >= 104500
-    bool result =
-        boost::spirit::qi::phrase_parse(first,
-                                        last,
-                                        g,
-                                        boost::spirit::ascii::space,
-                                        c);
-    if (!result)
-    {
-        throw config_error(std::string("Failed to parse color value: ") +
-                           "Expected a CSS color, but got '" + css_color + "'");
-    }
-#else
-    mapnik::css css_;
-    bool result =
-        boost::spirit::qi::phrase_parse(first,
-                                        last,
-                                        g,
-                                        boost::spirit::ascii::space,
-                                        css_);
-    if (!result)
-    {
-        throw config_error(std::string("Failed to parse color value: ") +
-                           "Expected a CSS color, but got '" + css_color + "'");
-    }
-    c.set_red(css_.r);
-    c.set_green(css_.g);
-    c.set_blue(css_.b);
-    c.set_alpha(css_.a);
-#endif
-}
-
-bool color_factory::parse_from_string(color & c, std::string const& css_color,
-                                      mapnik::css_color_grammar<std::string::const_iterator> const& g)
-{
-    std::string::const_iterator first = css_color.begin();
-    std::string::const_iterator last =  css_color.end();
-    // boost 1.41 -> 1.44 compatibility, to be removed in mapnik 2.1 (dane)
-#if BOOST_VERSION >= 104500
-    bool result =
-        boost::spirit::qi::phrase_parse(first,
-                                        last,
-                                        g,
-                                        boost::spirit::ascii::space,
-                                        c);
-    return result && (first == last);
-#else
-    mapnik::css css_;
-    bool result =
-        boost::spirit::qi::phrase_parse(first,
-                                        last,
-                                        g,
-                                        boost::spirit::ascii::space,
-                                        css_);
-    if (result && (first == last))
-    {
-        c.set_red(css_.r);
-        c.set_green(css_.g);
-        c.set_blue(css_.b);
-        c.set_alpha(css_.a);
-        return true;
-    }
-    return false;
-#endif
-}
-
-
-color color_factory::from_string(std::string const& css_color)
-{
-    color c;
-    init_from_string(c, css_color);
-    return c;
-}
-
 }
 

src/color_factory.cpp

@@ -0,0 +1,76 @@
+/*****************************************************************************
+ *
+ * This file is part of Mapnik (c++ mapping toolkit)
+ *
+ * Copyright (C) 2011 Artem Pavlenko
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ *****************************************************************************/
+
+// mapnik
+#include <mapnik/color.hpp>
+#include <mapnik/color_factory.hpp>
+#include <mapnik/config_error.hpp>
+#include <mapnik/css_color_grammar.hpp>
+
+// boost
+#include <boost/format.hpp>
+#include <boost/version.hpp>
+
+// stl
+#include <sstream>
+
+
+namespace mapnik {
+
+color parse_color(std::string const& str)
+{
+    css_color_grammar<std::string::const_iterator> g;
+    return parse_color(str, g);
+}
+
+color parse_color(std::string const& str,
+                  css_color_grammar<std::string::const_iterator> const& g)
+{
+    color c;
+
+    std::string::const_iterator first = str.begin();
+    std::string::const_iterator last =  str.end();
+    
+    // boost 1.41 -> 1.44 compatibility, to be removed in mapnik 2.1 (dane)
+#if BOOST_VERSION >= 104500
+    bool result = boost::spirit::qi::phrase_parse(first, last, g,
+                                                  boost::spirit::ascii::space,
+                                                  c);
+#else
+    mapnik::css css_;
+    bool result = boost::spirit::qi::phrase_parse(first, last, g,
+                                                  boost::spirit::ascii::space, 
+                                                  css_);
+    c.set_red(css_.r);
+    c.set_green(css_.g);
+    c.set_blue(css_.b);
+    c.set_alpha(css_.a);
+    
+#endif
+
+    if (result && (first == last))
+        return c;
+    else
+        throw config_error( "Failed to parse color: \"" + str + "\"" );
+}
+
+}

src/svg/svg_parser.cpp

@@ -74,7 +74,7 @@ agg::rgba8 parse_color(const char* str)
     mapnik::color c(100,100,100);
     try
     {
-        mapnik::color_factory::init_from_string(c,str);
+        c = mapnik::parse_color(str);
     }
     catch (mapnik::config_error & ex)
     {
@@ -784,7 +784,7 @@ void svg_parser::parse_gradient_stop(xmlTextReaderPtr reader)
             {
                 try
                 {
-                    mapnik::color_factory::init_from_string(stop_color,kv.second.c_str());
+                    stop_color = mapnik::parse_color(kv.second.c_str());
                 }
                 catch (mapnik::config_error & ex)
                 {
@@ -804,7 +804,7 @@ void svg_parser::parse_gradient_stop(xmlTextReaderPtr reader)
     {
         try
         {
-            mapnik::color_factory::init_from_string(stop_color,(const char *) value);
+            stop_color = mapnik::parse_color((const char *) value);
         }
         catch (mapnik::config_error & ex)
         {

src/xml_tree.cpp

@@ -86,15 +86,7 @@ inline boost::optional<std::string> fast_cast(xml_tree const& tree, std::string
 template <>
 inline boost::optional<color> fast_cast(xml_tree const& tree, std::string const& value)
 {
-    mapnik::color c;
+    return parse_color(value, tree.color_grammar);
-    if (mapnik::color_factory::parse_from_string(c, value, tree.color_grammar))
-    {
-        return c;
-    }
-    else
-    {
-        throw config_error("Failed to parse color '"+value+"'");
-    }
 }
 
 template <>

tests/cpp_tests/svg_renderer_tests/combined_test.cpp

@@ -30,7 +30,7 @@ BOOST_AUTO_TEST_CASE(combined_test_case)
     typedef svg_renderer<std::ostream_iterator<char> > svg_ren;
 
     Map map(800, 600);
-    map.set_background(color_factory::from_string("white"));
+    map.set_background(parse_color("white"));
 
     std::ostringstream output_stream;
     std::ostream_iterator<char> output_stream_iterator(output_stream);

tests/cpp_tests/svg_renderer_tests/file_output_test.cpp

@@ -43,7 +43,7 @@ BOOST_AUTO_TEST_CASE(file_output_test_case)
     typedef svg_renderer<std::ostream_iterator<char> > svg_ren;
 
     Map map(800, 600);
-    map.set_background(color_factory::from_string("blue"));
+    map.set_background(parse_color("blue"));
 
     std::string output_filename = "file_output_test_case.svg";
     std::ofstream output_stream(output_filename.c_str());

tests/cpp_tests/svg_renderer_tests/path_element_test.cpp

@@ -220,7 +220,7 @@ void render_to_file(Map const& m, const std::string output_filename)
 BOOST_AUTO_TEST_CASE(path_element_test_case_1)
 {
     Map m(800,600);
-    m.set_background(color_factory::from_string("steelblue"));
+    m.set_background(parse_color("steelblue"));
 
     prepare_map(m);