started unit test for Object, fixed possible used of already freed ArgumentValidator when copying ValidatingFunction

git-svn-id: file:///var/local/svn/basicwriter@77 daaaf23c-2e50-4459-9457-1e69db5a47bf

4 files changed, 133 insertions, 111 deletions

diff --git a/src/core/script/Function.cpp b/src/core/script/Function.cpp
index ce6e6b6..84473a8 100644
--- a/src/core/script/Function.cpp
+++ b/src/core/script/Function.cpp
@@ -21,8 +21,10 @@
 namespace ousia {
 namespace script {
 
-std::pair<bool, std::vector<Variant>> ArgumentValidator::setError(int idx,
-		const std::string &msg, std::vector<Variant> &res)
+/* Class ArgumentValidator */
+
+std::pair<bool, std::vector<Variant>> ArgumentValidator::setError(
+    int idx, const std::string &msg, std::vector<Variant> &res)
 {
 	errorIndex = idx;
 	errorMessage = msg;
@@ -36,7 +38,7 @@ void ArgumentValidator::resetError()
 }
 
 std::pair<bool, std::vector<Variant>> ArgumentValidator::validate(
-		const std::vector<Variant> &args)
+    const std::vector<Variant> &args)
 {
 	std::vector<Variant> res;
 
@@ -44,36 +46,56 @@ std::pair<bool, std::vector<Variant>> ArgumentValidator::validate(
 	resetError();
 
 	// Sanity check: Do not allow too many arguments
-	if (args.size() > descriptors.size()) {
-		return setError(descriptors.size(), "Expected " + std::to_string(descriptors.size()) +
-				" arguments but got " + std::to_string(args.size()), res);
+	if (args.size() > signature.size()) {
+		return setError(signature.size(),
+		                "Expected " + std::to_string(signature.size()) +
+		                    " arguments but got " + std::to_string(args.size()),
+		                res);
 	}
 
 	// Iterate over the given arguments and check their type
-	res.reserve(descriptors.size());
+	res.reserve(signature.size());
 	for (unsigned int i = 0; i < args.size(); i++) {
 		// TODO: Implicit type conversion
 		const VariantType tGiven = args[i].getType();
-		const VariantType tExpected = descriptors[i].type;
+		const VariantType tExpected = signature[i].type;
 		if (tGiven != tExpected) {
-			return setError(i, std::string("Expected type ") + Variant::getTypeName(tExpected)
-				+ " but got " + Variant::getTypeName(tGiven), res);
+			return setError(i, std::string("Expected type ") +
+			                       Variant::getTypeName(tExpected) +
+			                       " but got " + Variant::getTypeName(tGiven),
+			                res);
 		}
 		res.push_back(args[i]);
 	}
 
 	// Make sure the remaining arguments have a default value, and if they have
 	// one, add it to the result
-	for (unsigned int i = args.size(); i < descriptors.size(); i++) {
-		if (!descriptors[i].hasDefault) {
+	for (unsigned int i = args.size(); i < signature.size(); i++) {
+		if (!signature[i].hasDefault) {
 			return setError(i, "Expected argument " + std::to_string(i), res);
 		}
-		res.push_back(descriptors[i].defaultValue);
+		res.push_back(signature[i].defaultValue);
 	}
 
 	return std::make_pair(true, res);
 }
 
+/* Class ValidatingFunction */
+
+Variant ValidatingFunction::call(const std::vector<Variant> &args) const
+{
+	if (validate) {
+		ArgumentValidator validator{signature};
+
+		std::pair<bool, std::vector<Variant>> res = validator.validate(args);
+		if (!res.first) {
+			throw validator.error();
+		}
+		return validatedCall(res.second);
+	}
+
+	return validatedCall(args);
+}
 }
 }
 
diff --git a/src/core/script/Function.hpp b/src/core/script/Function.hpp
index 43afd3d..83160e4 100644
--- a/src/core/script/Function.hpp
+++ b/src/core/script/Function.hpp
@@ -43,9 +43,7 @@ public:
 	/**
 	 * Virtual destructor.
 	 */
-	virtual ~Function()
-	{
-	}
+	virtual ~Function() {}
 
 	/**
 	 * Abstract function which is meant to call the underlying function (be it
@@ -62,10 +60,7 @@ public:
 	 *
 	 * @return a Variant containing the return value.
 	 */
-	Variant call() const
-	{
-		return call({});
-	}
+	Variant call() const { return call({}); }
 
 	// TODO: Use () operator instead of the call function
 };
@@ -98,10 +93,7 @@ public:
 	ArgumentValidatorError(int index, const std::string &msg)
 	    : index(index), msg(msg){};
 
-	virtual const char *what() const noexcept override
-	{
-		return msg.c_str();
-	}
+	virtual const char *what() const noexcept override { return msg.c_str(); }
 };
 
 /**
@@ -113,7 +105,7 @@ private:
 	/**
 	 * List containing the argument descriptors.
 	 */
-	const std::vector<Argument> descriptors;
+	const std::vector<Argument> signature;
 
 	/**
 	 * Argument index in the input array, at which the last error occured.
@@ -125,21 +117,28 @@ private:
 	 */
 	std::string errorMessage;
 
+	/**
+	 * Used internally to update the errorIndex and the errorMessage fields.
+	 */
 	std::pair<bool, std::vector<Variant>> setError(int idx,
 	                                               const std::string &msg,
 	                                               std::vector<Variant> &res);
 
+	/**
+	 * Resets the error state.
+	 */
 	void resetError();
 
 public:
+
 	/**
 	 * Constructor of the argument validator class.
 	 *
 	 * @param descriptors is a list of Arguments which should be used
 	 * for the validation.
 	 */
-	ArgumentValidator(const std::vector<Argument> &descriptors)
-	    : descriptors(descriptors)
+	ArgumentValidator(const std::vector<Argument> &signature)
+	    : signature(signature)
 	{
 	}
 
@@ -177,40 +176,31 @@ public:
  */
 class ValidatingFunction : public Function {
 private:
-	ArgumentValidator *validator;
+	/**
+	 * Specifies whether the validating function should actually run or just
+	 * pass the arguments through.
+	 */
+	bool validate;
+
+	/**
+	 * Signature for which the function should be validated.
+	 */
+	const std::vector<Argument> signature;
 
 protected:
 	virtual Variant validatedCall(const std::vector<Variant> &args) const = 0;
 
-	virtual Variant call(const std::vector<Variant> &args) const override
-	{
-		if (validator) {
-			std::pair<bool, std::vector<Variant>> res =
-			    validator->validate(args);
-			if (!res.first) {
-				throw validator->error();
-			}
-			return validatedCall(res.second);
-		}
-		return validatedCall(args);
-	}
+	virtual Variant call(const std::vector<Variant> &args) const override;
 
 	using Function::call;
 
 public:
-	ValidatingFunction() : validator(nullptr)
-	{
-	}
+	ValidatingFunction() : validate(false) {}
 
-	ValidatingFunction(std::vector<Argument> signature)
-	    : validator(new ArgumentValidator(signature))
+	ValidatingFunction(const std::vector<Argument> &signature)
+	    : validate(true), signature(signature)
 	{
 	}
-
-	~ValidatingFunction() override
-	{
-		delete validator;
-	}
 };
 
 using HostFunctionCallback = Variant (*)(const std::vector<Variant> &args,
@@ -242,10 +232,7 @@ public:
 	{
 	}
 
-	Function *clone() const override
-	{
-		return new HostFunction(*this);
-	}
+	Function *clone() const override { return new HostFunction(*this); }
 
 	using ValidatingFunction::call;
 };
@@ -272,25 +259,13 @@ public:
 	      callback(callback),
 	      data(data){};
 
-	Function *clone() const override
-	{
-		return new Getter(*this);
-	}
+	Function *clone() const override { return new Getter(*this); }
 
-	Variant call() const
-	{
-		return ValidatingFunction::call();
-	}
+	Variant call() const { return ValidatingFunction::call(); }
 
-	Variant operator()() const
-	{
-		return call();
-	}
+	Variant operator()() const { return call(); }
 
-	bool exists()
-	{
-		return callback != nullptr;
-	}
+	bool exists() const { return callback != nullptr; }
 };
 
 class Setter : public ValidatingFunction {
@@ -316,25 +291,13 @@ public:
 	      callback(callback),
 	      data(data){};
 
-	Function *clone() const override
-	{
-		return new Setter(*this);
-	}
+	Function *clone() const override { return new Setter(*this); }
 
-	void call(Variant arg) const
-	{
-		ValidatingFunction::call({arg});
-	}
+	void call(Variant arg) const { ValidatingFunction::call({arg}); }
 
-	void operator()(Variant arg) const
-	{
-		return call(arg);
-	}
+	void operator()(Variant arg) const { return call(arg); }
 
-	bool exists()
-	{
-		return callback != nullptr;
-	}
+	bool exists() const { return callback != nullptr; }
 };
 }
 }
diff --git a/src/core/script/Object.cpp b/src/core/script/Object.cpp
index 8d858bc..8fa9771 100644
--- a/src/core/script/Object.cpp
+++ b/src/core/script/Object.cpp
@@ -21,63 +21,90 @@
 namespace ousia {
 namespace script {
 
-bool Object::hasElement(std::string name) const
+bool Object::hasElement(const std::string &name) const
 {
 	return (properties.find(name) != properties.end()) ||
 	       (methods.find(name) != methods.end());
 }
 
-void Object::addProperty(std::string name, const Property &property)
+void Object::addProperty(const std::string &name, const Property &property)
 {
 	if (hasElement(name)) {
 		// TODO Throw another exception class here
 		throw "Element already exists";
 	}
-	properties.emplace(name, property);
+	properties.insert(std::make_pair(name, property));
 }
 
-void Object::addProperty(std::string name, const Getter &get, const Setter &set)
+void Object::addProperty(const std::string &name, const Getter &get, const Setter &set)
 {
 	addProperty(name, Property{get, set});
 }
 
-void Object::addProperty(std::string name, VariantType type,
+void Object::addProperty(const std::string &name, VariantType type,
                          const GetterCallback get, const SetterCallback set)
 {
 	addProperty(name, Property{type, get, set, data});
 }
 
-void Object::addReadonlyProperty(std::string name, const Getter &get)
+void Object::addReadonlyProperty(const std::string &name, const Getter &get)
 {
 	addProperty(name, Property{get, Setter{VariantType::null, nullptr}});
 }
 
-void Object::addReadonlyProperty(std::string name, const GetterCallback get)
+void Object::addReadonlyProperty(const std::string &name, const GetterCallback get)
 {
 	addProperty(
 	    name, Property{Getter{get, data}, Setter{VariantType::null, nullptr}});
 }
 
-void Object::addMethod(std::string name, const HostFunction &fun)
+void Object::addMethod(const std::string &name, const HostFunction &fun)
 {
 	if (hasElement(name)) {
 		// TODO Throw another exception class here
 		throw "Element already exists";
 	}
-	methods.emplace(name, fun);
+	methods.insert(std::make_pair(name, fun));
 }
 
-void Object::addMethod(std::string name, const HostFunctionCallback fun)
+void Object::addMethod(const std::string &name, const HostFunctionCallback fun)
 {
 	addMethod(name, HostFunction{fun, data});
 }
 
-void Object::addMethod(std::string name, const HostFunctionCallback fun,
+void Object::addMethod(const std::string &name, const HostFunctionCallback fun,
                        const std::vector<Argument> &signature)
 {
 	addMethod(name, HostFunction{fun, signature, data});
 }
 
+const Property *Object::getProperty(const std::string &name) const
+{
+	auto it = properties.find(name);
+	return (it != properties.end()) ? &(it->second) : nullptr;
+}
+
+const Function *Object::getMethod(const std::string &name) const
+{
+	auto it = methods.find(name);
+	return (it != methods.end()) ? &(it->second) : nullptr;
+}
+
+bool Object::removeElement(const std::string &name)
+{
+	return removeProperty(name) || removeMethod(name);
+}
+
+bool Object::removeProperty(const std::string &name)
+{
+	return properties.erase(name) > 0;
+}
+
+bool Object::removeMethod(const std::string &name)
+{
+	return methods.erase(name) > 0;
+}
+
 }
 }
 
diff --git a/src/core/script/Object.hpp b/src/core/script/Object.hpp
index 20e0f0f..fafe632 100644
--- a/src/core/script/Object.hpp
+++ b/src/core/script/Object.hpp
@@ -38,7 +38,7 @@ struct Property {
 	 * @param get is the getter that should be used for the property.
 	 * @param set is the setter that should be used for the property.
 	 */
-	Property(const Getter &get, const Setter &set) : get(get), set(set){};
+	Property(const Getter &get, const Setter &set) : get(get), set(set) {};
 
 	/**
 	 * Constructor of the Property struct. Creates new Getter and Setter
@@ -51,7 +51,7 @@ struct Property {
 	 */
 	Property(VariantType type, const GetterCallback get,
 	         const SetterCallback set, void *data = nullptr)
-	    : get(get, data), set(type, set, data){};
+	    : get(get, data), set(type, set, data) {};
 
 	/**
 	 * Getter function.
@@ -92,35 +92,45 @@ private:
 
 public:
 	Object() : data(nullptr){};
-
+	
 	Object(void *data) : data(data){};
 
-	bool hasElement(std::string name) const;
+	bool hasElement(const std::string &name) const;
 
-	void addProperty(std::string name, const Property &property);
+	void addProperty(const std::string &name, const Property &property);
 
-	void addProperty(std::string name, const Getter &get, const Setter &set);
+	void addProperty(const std::string &name, const Getter &get, const Setter &set);
 
-	void addProperty(std::string name, VariantType type,
+	void addProperty(const std::string &name, VariantType type,
 	                 const GetterCallback get, const SetterCallback set);
 
-	void addReadonlyProperty(std::string name, const Getter &get);
+	void addReadonlyProperty(const std::string &name, const Getter &get);
 
-	void addReadonlyProperty(std::string name, const GetterCallback get);
+	void addReadonlyProperty(const std::string &name, const GetterCallback get);
 
-	void addMethod(std::string name, const HostFunction &fun);
+	void addMethod(const std::string &name, const HostFunction &fun);
 
-	void addMethod(std::string name, const HostFunctionCallback fun);
+	void addMethod(const std::string &name, const HostFunctionCallback fun);
 
-	void addMethod(std::string name, const HostFunctionCallback fun,
+	void addMethod(const std::string &name, const HostFunctionCallback fun,
 	               const std::vector<Argument> &signature);
 
-	const std::map<std::string, Property> &getProperties()
+	const Property *getProperty(const std::string &name) const;
+
+	const Function *getMethod(const std::string &name) const;
+
+	bool removeElement(const std::string &name);
+
+	bool removeProperty(const std::string &name);
+
+	bool removeMethod(const std::string &name);
+
+	const std::map<std::string, Property> &getProperties() const
 	{
 		return properties;
 	}
 
-	const std::map<std::string, HostFunction> &getMethods()
+	const std::map<std::string, HostFunction> &getMethods() const
 	{
 		return methods;
 	}