cristi1990an / tensor-plus-plus Goto Github PK

From your comment on #3 here are some suggestions,

Project layout

1. please provide a .clang-format file into the repo and use clang-format, it will help contribution
2. organize your files, make tests/src/include directories
3. use more tools, like sanitizers/clang-tidy...
4. you might want to use a testing library (e.g.: boost test, google test, ...)
5. watch out for your repo, there is a lot of duplicated commits, and strange merge from master, maybe get a better understanding of how Git works: https://git-scm.com/book/en/v2
   • you can use git revert for some or git reset/git commit --amend if it's not pushed
6. you should use a better name for 'helpers.hpp' as it is just IO (generally avoid generic names like helper, utils, controller...)

On the code itself:

7. you are using redundant == true and == false (clang-tidy is giving me this warning btw, you'd have to activate readability checks)
8. be consistent on the use of ! or not pick one (! is more common)
9. I am not a big fan of loops and ifs without blocks {} to avoid problems and be consistent
10. param_1 You might want to name those (and also TEST_1...)
11. generally avoid using raw new/delete it's not exception safe, etc, and in this case I think you could use std::swap_ranges, and why are you making copies?

    Tensor-plus-plus/tensor.hpp

    Line 1275 in 5c902ac

    T* aux = new T[size];

12. also, you could factorize your duplicated code, like for instance, call the lvalue swap simply like swap(left, right) in the rvalue overload, it works because the rvalue has been named in the function:

    Tensor-plus-plus/tensor.hpp

    Line 1388 in 25de86a

    void swap(subdimension<T, Rank, allocator_type>&& left, subdimension<T, Rank, allocator_type>&& right)

13. if you haven't watched it already, here is a great talk on standard algorithms: https://www.youtube.com/watch?v=2olsGf6JIkU, generally CppCon is great and freely available on youtube :)
14. avoid using reinterpret_cast and read carefully the rules if you don't know them already (it's always good to reread them: https://en.cppreference.com/w/cpp/language/reinterpret_cast), moreover here you could just call _data.data() or to be more generic std::data(_data):

    Tensor-plus-plus/tensor.hpp

    Line 1074 in 25de86a

    return reinterpret_cast<T*>(&it);

15. if you want to be more generic, you should prefer free function for begin/end/data/size/... (std::begin/std::end/... instead of methods)
16. swap and move operations should not throw
17. I think you might want to follow more the rule of 0: https://en.cppreference.com/w/cpp/language/rule_of_three (e.g.: for your iterators)
18. you should use std::equal with 4 iterators instead of 3 to do bound checking (I see that by design they have the same size, you should put comments at least then, there is also == it should work, maybe create a span with a templated size) minor issue
19. This would probably better be some kind of assert, this seems to be a logic error:

    Tensor-plus-plus/tensor.hpp

    Line 1326 in 8983300

    throw std::runtime_error("Can't swap subdimensions of different sizes!\n");

20. Choose to use (void) or () and stay consistent:

    Tensor-plus-plus/tensor.hpp

    Line 1070 in 8983300

    reference operator* (void) const noexcept

I might not have understood all your math/thinking/vision. Also I haven't done a detailed review.

Great work, Hope that'll help :)

If you change/add your sources (.cpp) you should change your CMakeLists.txt go see it, it's not a very complex one

you can read the doc for more details:

• https://cmake.org/cmake/help/latest/guide/tutorial/index.html
• https://www.youtube.com/watch?v=_yFPO1ofyF0&list=PLK6MXr8gasrGmIiSuVQXpfFuE1uPT615s

for more advance stuff, there is a lot of good talk:

• https://www.youtube.com/watch?v=bsXLMQ6WgIk

and here for an awesome list: https://github.com/onqtam/awesome-cmake

Hi, Nihili0 from reddit here

Just to let you know before you hit cryptic compiler error

you can't specialize functions on untemplated arguments

you can do:

struct S {
  template<typename T>
  void f(T const& t) {
  }

  template<>
  void f(int const& i) {
  }
};

but you can't do:

template<typename T>
struct S {
  template<typename U>
  void f(U const& u) {
  }

  template<>
  void f(int i) { // error: not a const&
  }

  template<std::size_t R>
  void f(useful_specializations::nested_initializer_list<T, R> const&) {
  }

  template <>
  constexpr void construct_order_array<1u>(const std::initializer_list<T>& data) { // error: not specializing on templated type
  }
};

Trying to remove all warnings that popped up