Make JavaScript sources compile into the library; convert JavaScript files on the fly

include/AdblockPlus/JsEngine.h

 #ifndef ADBLOCKPLUS_JS_ENGINE_H
 #define ADBLOCKPLUS_JS_ENGINE_H
 
 #include <stdexcept>
 #include <string>
 #include <v8.h>
 
 class ErrorCallback;
 class FileReader;
 
 namespace AdblockPlus
 {
   class JsError : public std::runtime_error
   {
   public:
     JsError(const v8::Handle<v8::Value> exception);
   };
 
   class JsEngine
   {
   public:
     JsEngine(const FileReader* const fileReader,
              ErrorCallback* const errorCallback);
     void Evaluate(const char* source, const char* filename = NULL);

[Felix Dahlke, 2013/03/15 15:18:36]

Have you considered having just one Evaluate function

void Evaluate(const std::string& source, const std::string& filename = "")

and checking for "" instead of NULL?

[Wladimir Palant, 2013/03/15 16:00:41]

I would rather use char* for the main function - we have our JS sources as char*
and that's what we are passing to V8, converting to std::string in between
sounds pointless. Still, I can probably merge the two Evaluate functions using
std::string.

[Felix Dahlke, 2013/03/15 17:10:47]

It's up to you. std::string is obviously more C++-ish than const char*, and this
wouldn't be the only occasion where we (well, I, so far :D) wrap Google's C-y
API in a more C++-y way. The conversion overhead between char* and std::string
is neglectable in most cases.

-    void Evaluate(const std::string& source)

[Felix Dahlke, 2013/03/15 15:18:36]

I prefer to have all implementations in the .cpp file, but it's of course up to
you. However, this will lead to a compile error if included more than once as it
stands, you need to use the inline keyword for functions defined in headers.

[Wladimir Palant, 2013/03/15 16:00:41]

Yes, I tend to be a bit sloppy about that - fair point.

-    {
-      Evaluate(source.c_str());
-    }
-    void Evaluate(const std::string& source, const std::string& filename)
-    {
-      Evaluate(filename.c_str(), source.c_str());
-    }
+    void Evaluate(const std::string& source,
+        const std::string& filename = "");
     void Load(const std::string& scriptPath);
     std::string Call(const std::string& functionName);
     void Gc();
 
   private:
     const FileReader* const fileReader;
     v8::Persistent<v8::Context> context;
-    inline v8::Handle<v8::Script> CompileScript(const char* source, const char* filename);

[Felix Dahlke, 2013/03/15 15:18:36]

You probably don't want to declare this inline. It doesn't guarantee inlining,
and you'll have to supply an identical definition in all translation units.

As a rule of thumb, it pretty much only makes sense to declare functions defined
in the header as inline (see above).

   };
 }
 
 #endif