htm-community / htm.core

Actively developed Hierarchical Temporal Memory (HTM) community fork (continuation) of NuPIC. Implementation for C++ and Python
http://numenta.org
GNU Affero General Public License v3.0
148 stars 74 forks source link

STL compatible size(), dimensions() return size_t #357

Open breznak opened 5 years ago

breznak commented 5 years ago

A coding-style proposal: our new code will be STL compatible and size(), dimensions(), capacity, etc will return size_t value;

Current: we use UInt or certain type that is used in the container (eg vector<char>()::size() we'd cast to char.

New: STL uses size_type (size_t, ulong)

Motivation: in PR #331 I'm playing with changing back-end data-types for containers. We get incompatibilities with other parts in the code on each change, deciding on size_t would fix this.

ctrl-z-9000-times commented 5 years ago

This sounds great. Its always nice to be compatible with the standard library.

dkeeney commented 5 years ago

I agree. Anything related to size ... or indexes for that matter, should be size_t. There is a nupic::Size type but it is not consistently used.

breznak commented 5 years ago

There is a nupic::Size type but it is not consistently used.

good point, should we use nupic::Size (i think now typedef'ed to size_t), or just drop it and deal with size_t only?

dkeeney commented 5 years ago

or just drop it and deal with size_t only?

Well, for what it is worth, Cereal uses an unsigned long long int for size rather than size_t. But I think that is a spacial case and everything else uses size_t.

Let's just use size_t.

breznak commented 5 years ago

from https://github.com/htm-community/nupic.cpp/pull/372#issuecomment-480546242 , @dkeeney

To be consistent....we should make ALL size related things size_t. This includes the dimensions.

yes, although I'd like to advocate against that.