Puara / puara-gestures

High-level gestural descriptor functions for the Puara Framework
MIT License
1 stars 6 forks source link

Refactor and improve code #17

Closed jcelerier closed 1 week ago

jcelerier commented 4 weeks ago
class Wrap
{
public:
  double min{};
  double max{};
  void f() { blabla; }
};

one can simply create it like:

Wrap w1{-100., 456.};

or even better

Wrap w2{.min = -100., .max = 456.};

C++20 also allows to use () instead of {} for this but I think it is better to use {} as it will refuse to do any conversion that would loose precision unlike () which happily does a ton of implicit conversions)

edumeneses commented 4 weeks ago

Thanks for that work @jcelerier.

We can merge as is if it's working (I didn't test it).

However, I'm having second thoughts about creating a utils folder and separating the classes as we did with the descriptors. What do you think?

jcelerier commented 4 weeks ago

yes, great idea ! I'll do that

jcelerier commented 4 weeks ago

Ok, did a much more thorough cleanup and adapted the library structure to match more closely current expectations and standards in the C++ community

jcelerier commented 4 weeks ago

Ok, ci builds on Mac/Win/Linux I think we can safely merge :) I noticed in the refactor that some functions disappeared though, added a few fixme in the code to bring them back to life @samamou

jcelerier commented 1 week ago

ty!

edumeneses commented 1 week ago

@jcelerier and @samamou , I noticed the refactored files don't have the latest editor name.

Perhaps adding you/them to the head comments on each descriptor/utils file is a good idea?

jcelerier commented 1 week ago

hmm to be honest my take on this is that this information is part of git history anyways - adding it to the source files is not adding much value to me. Likewise for the license file, usually at most nowadays we just add SPDX identifiers: https://spdx.github.io/spdx-spec/v2.3/using-SPDX-short-identifiers-in-source-files/

edumeneses commented 1 week ago

@jcelerier , do we wait for @samamou 's review?

samamou commented 1 week ago

I think i will need to check for the functions that got removed when we were refactoring, I have some time tomorrow @edumeneses

jcelerier commented 1 week ago

build is broken :^)

jcelerier commented 1 week ago

all is green, ACK for me to merge ! congrats :)

jcelerier commented 1 week ago

Edu is it good for you ?

jcelerier commented 1 week ago

\o/