Review type definitions

[tap]

We have several cases for which we need to review our code to clean up our type definitions:

• pointer/ref variants should be eliminated (e.g. TTMutexPtr and TTMutexRef). We want to discourage the use of pointers and references, and purge our own use of them where possible, so there should not be typedefs that encourage or hide their use.
• there are types we essentially never use (e.g. TTImmutableCString) so we shouldn't be defining them
• we have types of questionable use. For example TTInt8 and TTComplex. Unfortunately, this type is actually used, but in my first pass of reviewing it the use seems questionable and could/should(?) be replaced with use by a normal-sized int. The exception is TTMatrix. But there are other types like this. Types like this actually slow down execution at runtime because of branching on types in various conversion routines.
• question: do we want to use TTBoolean instead of the standard bool? TTInt32 instead of the standard int? That's what we've been doing, but is that really desirable?
• some types should be scoped to their context, e.g. TTRowID is for Matrix use only. So it should be in the Matrix code not in TTBase.h
• eliminate iterator typedefs -- these are not idiomatic for C++ coding and furthermore should rarely be needed anyway thanks to "auto" vars in C++11.
• Function Pointers...
• Use C++11/14 alias declarations instead of the old-sk00l typedef

[nwolek]

Tim:
Thanks for reviewing this. Frankly, this has felt like a moving target for a while to me. It would be good to get this sorted out sooner rather than later. Do you see this as a higher priority than the Jamoma Graph discussion you initiated recently?

Surveying the topics in this "Foundation Code Review" milestone begs the question: what is realistic to get done before the Florida workshop? What must/should wait until we can have extended real-time discussions?

-Nathan

[tap]

Hi @nwolek -- the answer to your question is "it depends", in part because I'm still trying to evaluate our current state of affairs.

The upcoming workshop is a good motivator. One of the big topics for the workshop is addressing the inadequacies of the AudioGraph. However, without dealing with some of the underlying plumbing we essentially will be wasting our time at the workshop. So we need to get our brain cells on the Graph immediately so we can show up prepared.

This particular ticket may impact the graph somewhat -- due to some of these seemingly cosmetic issues we actually have places in the code that leak resources and ultimately corrupt the operation of the system.

Where this specific ticket has larger bearing is on integration with something like OpenFrameworks, which is another topic I've heard mentioned for the workshop. For our C++ library to play well with other C++ libraries it will be important to do some housekeeping to ensure that our code is not only correct, but also idiomatic and maintainable.

Finally, as may have already been obvious, my review has highlighted how little coverage we have with unit testing. See #342. This is disheartening. If we don't address this Jamoma will soon crumble under its own weight. We simply have too much code by too many authors. So again, without addressing this critically we will be wasting our time at the workshop.

As for what is "realistic" between now and July I am unsure. Reviewing the code is realistic. Once the review is complete then we should be able to prioritize the issues accordingly.

[tap]

More action oriented...

while reviewing iterator types and usage we should look at using const iterators (i.e. cbegin() and cend() instead of begin() and end()) where possible.

[nwolek]

Agreed. Sounds like getting things cleaned up in Core will need some focus for the time being. Next BoD should be interesting. You are more up to speed on best practices for C++, but if you find something you think you can explain to me and have me attack it, let me know.

[jcelerier]

About "TTInt32 instead of the standard int?" I'd argue that having the bits in the type is actually a good idea, since it solves a lot of problems when working on different processors (for instance while working with Jamoma on 32-bit ARM boards, "int" and "long" would not be the same size as 32-bit x86 CPUs, and it was nice to know what the actual size of the type would have been.)

However, prefixing it by TT seems a bit heavy to me; and I'd rather use the types defined in : http://en.cppreference.com/w/cpp/types/integer.

There is also the problem of the size of vectors, and of the correct way to make a "simple" for-loop with std::vector and friends. (A nice read on the subject are the comments on Herb Sutter's post : http://herbsutter.com/2013/06/13/gotw-93-solution-auto-variables-part-2/).

Finally, a good style recommendation now is also to use free-functions std::begin() and std::end() if necessary (since they work on C-style arrays too) ; however their const-friendly brothers std::cbegin() and std::cend() are only here since C++14, and sadly MSVC still lags a lot behind in this area (they are here only since VS2013 : http://blogs.msdn.com/b/vcblog/archive/2014/06/11/c-11-14-feature-tables-for-visual-studio-14-ctp1.aspx) hence it could be dangerous for the Windows build to use too much C++14 features in the short term.

[tap]

Thanks for the chart. MSVC has been a perpetual source of disappointment and frustration to me for over a decade, so I should not be very surprised.

FWIW, in the past Jamoma has been quick to adopt the latest compiler, version of Max, etc. since we have not reached 1.0 yet. So when VS2015 hits I personally have no misgivings about adopting it quickly.

As for type naming... I've not seen "int" change sizes in the last 20 years (it was smaller on 68K Mac machines prior to the PowerPC chip), though the size of "long" is very inconsistent. If we can use the standard (e.g. "int32") then is there an argument for keeping the TT type?