search

hameerabbasi / xsparse

XSparse is an experimental XTensor-like C++ sparse-array library.
BSD 3-Clause "New" or "Revised" License
9 stars 6 forks source link

Adding required member functions of abstract container traits #29

Closed adam2392 closed 1 year ago

adam2392 commented 1 year ago

Towards: #27

cc: @bharath2438 is this something along the lines of what you were thinking to add required API for a user defining their own container traits?

adam2392 commented 1 year ago

@hameerabbasi this is good to review now as well. I just add a few static asserts based on what we need from the Vec and Map in the levels.

One thing I noticed though is that Set is not used. Is this an issue?

bharath2438 commented 1 year ago

@adam2392 this looks good to me. I guess std::set has a contains function in C++20 so we might as well ensure there's an implementation for that as well.

adam2392 commented 1 year ago

@adam2392 this looks good to me. I guess std::set has a contains function in C++20 so we might as well ensure there's an implementation for that as well.

Kay added!

What would we even use Set for? It seems it is not used in the level formats?

bharath2438 commented 1 year ago

What would we even use Set for? It seems it is not used in the level formats?

std::unordered_set can be used for representing unordered levels

hameerabbasi commented 1 year ago

This LGTM overall. I'd just static assert these things in the container traits class itself, and make sure that the signature matches as well, rather than just the function existing. Otherwise looks excellent! Great work!