GothicKit/ZenKit

string_view version of `script::find_symbol_by_name` and few others

Try opened this issue · 2 comments

Try commented

Current syntax:
symbol* script::find_symbol_by_name(const std::string& name) forces client to bake std::string into interface.
for find function this is especially unnecessary, since it does string copy immediatly after:

	symbol* script::find_symbol_by_name(const std::string& name) {
		std::string up {name}; // <-- fine, but no need to query name as string
		std::transform(up.begin(), up.end(), up.begin(), ::toupper);

		if (auto it = _m_symbols_by_name.find(up); it != _m_symbols_by_name.end()) {
			return find_symbol_by_index(it->second);
		}

		return nullptr;
	}

In general it's probably would be good to pass read-only strings as string_view in all cases. If implementation then uses this string as key to has-map or similar - solve this locally inside the function.

Other places:

R vm::call_function(const std::string& name, P... args)
vm::init_instance(const std::string& name)
vm::allocate_instance(const std::string& name)
void vm::push_string(const std::string& value)
void register_external(const std::string& name, const std::function<R(P...)>& callback)
void override_function(const std::string& name, const std::function<R(P...)>& callback)
const way_point* way_net::waypoint(const std::string& name) const
const message_block* messages::block_by_name(const std::string& name) const

Partial duplicate of #21, but a good point. Until I can figure out a C++17 compatible way of using string-views all the way down, this is probably a good solution; especially for case-insensitive strings.

Since I've been able to implement most of these as std::string_view I'm closing this issue. I have deprecated waynet::waypoint since that API is broken anyways.