Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is there any pattern that can combine overload and override? #208

Closed
FlyinCow opened this issue Nov 28, 2024 · 3 comments
Closed

Is there any pattern that can combine overload and override? #208

FlyinCow opened this issue Nov 28, 2024 · 3 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@FlyinCow
Copy link

Say I have some c++ classes, each of them has a get and a set. They are abstractions of "attributes":

class Attribute{};

class IntAttribute: public Attribute{
public:
  int get(){return data;}
  void set(int value){data = value;}
private:
  int data;
};

class StringAttribute: public Attribute;

Naturally, different attribute types have different data types, therefore have different get and set (mostly T get() and void set(T)). I have to put them all together into a single container, so some polymorphism type is needed. But virtual functions can't help here because subclasses have different member function signatures.

Furthermore, to support arbitrary data type I have something like:

template<class T>
class TypedAttribute{
public:
  T get(){return data;}
  void set(T value){data = value;}
  template<class ...Args>
  void set(Args ...args){
    data = T{std::forward<Args>(args)...};
  }
private:
 T data;
};

Is there anything I can do with proxy to handle this situation? I can't figure out a way to dispatch get and set calls to subclasses. Currently I just dynamic convert a parent class into a subclass.

@mingxwa
Copy link
Collaborator

mingxwa commented Dec 2, 2024

@FlyinCow Thank you for asking. If I understand correctly, the semantics of your existing abstraction model relies on different types T, which is not directly supported by proxy without RTTI. Note that dynamic_cast is generally considered a bad practice. If IntAttribute or StringAttribute only serves for data storage, std::variant should be preferred. Please correct me if I missed anything.

However, we do have heard some feedback suggesting we add RTTI support (type_id, dynamic_cast) to proxy and allow users to opt-in. We are currently evaluating this feature request. Please let us know if you have any thoughts on this. Thank you! 😄

@mingxwa mingxwa added enhancement New feature or request question Further information is requested labels Dec 2, 2024
@FlyinCow
Copy link
Author

FlyinCow commented Dec 3, 2024

@FlyinCow Thank you for asking. If I understand correctly, the semantics of your existing abstraction model relies on different types T, which is not directly supported by proxy without RTTI. Note that dynamic_cast is generally considered a bad practice. If IntAttribute or StringAttribute only serves for data storage, std::variant should be preferred. Please correct me if I missed anything.

However, we do have heard some feedback suggesting we add RTTI support (type_id, dynamic_cast) to proxy and allow users to opt-in. We are currently evaluating this feature request. Please let us know if you have any thoughts on this. Thank you! 😄

The feature requested here is, formally, a generic facade, or an overloaded facade. Ideally I want to dispatch call by function name, but I don't think this is doable, or at least it's not easily to be done with both efficiency and type safety. In my production code I have a traditional tagged union like RAII wrapper (a unique_ptr of base class and an enum presents its type). Because all of my constructions are controlled by this wrapper, I don't need to care about type safety, and thus I could always perform a downward casting (whether static_cast or dynamic_cast, because I always know the actual type). I have found a junky way to make a united interface on this wrapper with if constexpr and is_invocable_v but I don't think it's the best practice, maybe something is wrong in my code design.

// This is a member function of the wrapper. `_attr` is the unique_ptr and `_type` is the enum.
// `if constexpr` here is to make these codes able to compile, and provide with a little safety.
template<class... Args>
void setValue(Args... args) {
        switch (_type) {
            case AttributeType::Int32:
                if constexpr (std::is_invocable_v<decltype(&Int32Attribute::setValue), Int32Attribute *, Args...>) {
                    static_cast<Int32Attribute &>(*_attr).setValue(std::forward<Args>(args)...);
                    break;
                } else {
                    throw something;
                }
            case AttributeType::UInt32:
                if constexpr (std::is_invocable_v<decltype(&UInt32Attribute::setValue), UInt32Attribute *, Args...>) {
                    static_cast<UInt32Attribute &>(*_attr).setValue(std::forward<Args>(args)...);
                } else {
                    throw something;
                }
                break;
        ......

The real reason I turn to proxy is I wonder if there is a better way to do such generic dispatch, in which case a facade may have a generic interface or some overloads of same interface. In virtual function-based polymorphism this is not doable:

struct D{
  virtual void interface();
  template<class T>
  void generic_interface(); // this could not be virtual
};

struct B: D{
  void interface() override; // It's nice, a `D*` pointer to a `new B` would call this
  void interface(int); // no way to call it from `D*` without casting, and override key word cannot be used here
};

If I use CRTP then Base<Derived> will be different types with different Derived so I can't fit them into one single container, and the overloaded functions are still not callable. I can always use a variant or an any though, but I'm seeking for some better way. I don't understand ProOverload so much, is this the thing I need?


BTW the current user guides and documentation is not so easy to read. The README covers the most common situations, but it seems there's no advanced usage? It would be nice if the specifications has a navigation bar. And is there any plan to make a talk about this great work, like on CPPCON?

@mingxwa
Copy link
Collaborator

mingxwa commented Dec 20, 2024

@FlyinCow Thank you for the explanation and sorry for the delayed response. Although I can't tell if the design of abstraction can be optimized with limited context of your project, you can use proxy to reduce the implementation of setValue with RTTI support today. For example:

#include <iostream>

#include "proxy.h"

struct CopyableObject : pro::facade_builder
    ::support_copy<pro::constraint_level::nontrivial>  // Support copy
    ::support_rtti  // Support proxy_cast, proxy_typeid
    ::build {};  // You can add whatever conventions you want

namespace details {

template <class T>
pro::proxy<CopyableObject> PolyGetValue(T&& self) {
  return pro::make_proxy<CopyableObject>(self.getValue());
}

template <class T>
void PolySetValue(T&& self, pro::proxy<CopyableObject> val) {
  using AttributeType = std::decay_t<decltype(std::declval<T>().getValue())>;
  self.setValue(proxy_cast<AttributeType>(std::move(*val)));
}

}  // namespace details

PRO_DEF_FREE_AS_MEM_DISPATCH(MemPolyGetValue, details::PolyGetValue, getValue);
PRO_DEF_FREE_AS_MEM_DISPATCH(MemPolySetValue, details::PolySetValue, setValue);

struct Attribute : pro::facade_builder
    ::add_convention<MemPolyGetValue, pro::proxy<CopyableObject>()>
    ::add_convention<MemPolySetValue, void(pro::proxy<CopyableObject>)>
    ::build {};

class IntAttribute {
 public:
  IntAttribute(int value) : data(value) {}
  IntAttribute(const IntAttribute&) = default;
  int getValue() { return data; }
  void setValue(int value) { data = value; }

 private:
  int data;
};

int main() {
  std::shared_ptr<IntAttribute> attribute = std::make_shared<IntAttribute>(123);
  pro::proxy<Attribute> p = attribute;  // p holds a copy of attribute
  pro::proxy<CopyableObject> val = p->getValue();
  std::cout << proxy_typeid(*val).name() << "\n";
  proxy_cast<int&>(*val) = 456;
  p->setValue(std::move(val));
  std::cout << attribute->getValue() << "\n";  // Prints: "456"
}

Note that the code above won't compile with version 3.1 or earlier because support_rtti is only available on main branch today. This feature will be included in the upcoming 3.2 release. To make it compile with version 3.1, replacing pro::proxy<CopyableObject> with std::any should be feasible, but probably with different performance depending on the implementation. The documentation of support_rtti is still under development. It provides similar functionality like typeid and std::any_cast. Please let us know if this meets your requirements or there is anything missing.

BTW the current user guides and documentation is not so easy to read.

Your ideas to improve documentation of the library is much appreciated! Please feel free to file a separate issue or a pull request.

And is there any plan to make a talk about this great work, like on CPPCON?

Thank you for appreciating our work. We will consider your suggestion and maybe have some public talks next year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants