node-addon-api icon indicating copy to clipboard operation
node-addon-api copied to clipboard

Remove redundant string creation for PropertyLValue's _key when `object["bar"] = 1`

Open ShenHongFei opened this issue 2 years ago • 1 comments

Could we change

inline Object::PropertyLValue<string> Object::operator[](
    const char* utf8name) const {
  return PropertyLValue<string>(*this, utf8name);
}

to

inline Object::PropertyLValue<const char*> Object::operator[](
    const char* utf8name) const {
  return PropertyLValue<const char*>(*this, utf8name);
}

so that no redundant string will be created for PropertyLValue's _key?

template <typename Key>
class PropertyLValue {
public:
  /// Converts an L-value to a value.
  operator Value() const;

  /// Assigns a value to the property. The type of value can be
  /// anything supported by `Object::Set`.
  template <typename ValueType>
  PropertyLValue& operator =(ValueType value);

private:
  PropertyLValue() = delete;
  PropertyLValue(Object object, Key key);
  napi_env _env;
  napi_value _object;
  Key _key;

  friend class Napi::Object;
};

Subsequent Object::Set and napi_set_named_property also supports passing const char* directly

template <typename Key> template <typename ValueType>
inline Object::PropertyLValue<Key>& Object::PropertyLValue<Key>::operator =(ValueType value) {
#ifdef NODE_ADDON_API_ENABLE_MAYBE
  MaybeOrValue<bool> result =
#endif
      Object(_env, _object).Set(_key, value);
#ifdef NODE_ADDON_API_ENABLE_MAYBE
  result.Unwrap();
#endif
  return *this;
}
template <typename ValueType>
inline MaybeOrValue<bool> Object::Set(const char* utf8name,
                                      const ValueType& value) const {
  napi_status status =
      napi_set_named_property(_env, _value, utf8name, Value::From(_env, value));
  NAPI_RETURN_OR_THROW_IF_FAILED(_env, status, status == napi_ok, bool);
}

ShenHongFei avatar Mar 25 '22 13:03 ShenHongFei

@ShenHongFei that might break existing code right? Can we add versus replace in a way that will match char * if that is passed in but still allow an std:string to be passed in as well ?

mhdawson avatar May 10 '22 19:05 mhdawson