Another array length question

I know this seems to be a common issue, but I cant seem to find anyone addressing it for arrays passed as parameters in functions.
My students (Im a HS Computer teacher) are trying to get a 7seg led to countdown using arrays.

This is their abbreviated code:
int six []= {13,11,5,4,3,2};...

void setup(){
pinMode(13, OUTPUT);...
}

void loop(){
lightNum(six);...

}

void lightNum(int* lights){
for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)
digitalWrite(lights*, HIGH);*

  • delay(1000);*
  • for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)*
    _ digitalWrite(lights*, LOW);_
    _
    }*_
    Thanks community,
    JB

digitalWrite(lights, HIGH);

Why write the same value multiple times?

Did you mean

digitalWrite(lights[i], HIGH);

?

Abbreviated code is useless, please post all of it, using the # icon in the editor's toolbar.

void lightNum(int* lights){
  for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)
    digitalWrite(lights, HIGH);
  delay(1000);
  for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)
    digitalWrite(lights, LOW);
}

should be

void lightNum(int* lights)
{
  for(int i=0; i<(sizeof(lights)/sizeof(lights[0])); i++)    // if you decide to change the type of lights
    digitalWrite(lights[i], HIGH);
  delay(1000);
  for(int i=0; i<(sizeof(lights)/sizeof(lights[0])); i++)
    digitalWrite(lights[i], LOW);
}
void lightNum(int* lights)
{
  for(int i=0; i<(sizeof(lights)/sizeof(lights[0])); i++)

Hmm, kinda constant, no?

yep, will be calculated compile time and replaced by a fixed value.

The advantage is that when you decide to change the array element from int to long you don't have to change all those lines where you state

sizeof(lights)/sizeof(int) ==> sizeof(ligths)/sizeof(long)

Rob
PS, learned this the hard way too :wink:

yep, will be calculated compile time and replaced by a fixed value.

Yes, and it is the same constant, regardless of the array passed into "lightNum"
As far as "lightNum" is concerned, "lights" is an int pointer, and on an AVR an int pointer is 2 bytes longs.
So, sizeof (lights) is 2, and since "lights[0]" is an "int", so sizeof (lights[0]) is 2.
So 2/ 2 is 1.

pudding test ...

int x[20];

void setup()
{
  Serial.begin(115200);
}

void loop()
{
  Serial.println(sizeof(x)/sizeof(x[0]));
}

20 ...

Now try a scope test, with your original post.

int x[20];

void setup()
{
  Serial.begin(115200);
}

void loop()
{
  Serial.println(sizeof(x)/sizeof(x[0]));
  lightNum (x);
}

void lightNum(int* lights)
{
  Serial.println(sizeof(lights)/sizeof(lights[0]));
}

(what exactly is a "pudding test"?)

Ahyour right :frowning:
I allready said I learned it the hard way .... again :wink:

It'd be better to comment on complete code. Commenting based on fragments is an exercise in imaginative writing.

Entertaining at best but certainly not informative

void lightNum(int* lights){
  for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)
    digitalWrite(lights, HIGH);
  delay(1000);
  for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)
    digitalWrite(lights, LOW);
}

@lloyddean: what more needs commenting on?

int six []= {13,11,5,4,3,2};...
#define SIZEOF_SIX (sizeof (six)/sizeof (six[0]))

Better?

Well how about the fact that we may be "assuming" the array being passed in "six". Maybe it is, maybe it isn't.

If it is it's a global. And if so why pass the array as a parameter to the function "lightNum" when it's available for all to see?

But if your assumption is correct he might as well change "lightNum" to take no parameters and your latest suggestion is most useful.

C++ treats an array passed into a function (non global) as a pointer - thus why you reference it as int*. The size of an int pointer is always 2, as previously noted. Thus, you cannot use sizeof to find the length of an array. You therefore have two alternatives:

  1. Pass the array length in separately, as a parameter to the function (this is relatively common)
  2. Use a termination value at the end of the array ('\0' is the null character and is used for strings)
    -> note that this means you can also use a char* with a null-termination, and use C++ string methods, such as strlen.

Of course, if the array is global, you don't have these issues =).

Well how about the fact that we may be "assuming" the array being passed in "six". Maybe it is, maybe it isn't

void loop(){
lightNum(six);...

}

void lightNum(int* lights){

How is it global?

Alright, here is the entire code:
The idea is to countdown on a 7 segment. They got it to work by sending the size of the array down each time it was called, but we are wondering where we went wrong, it seems to me that had we done:
for(int i=0; i<(sizeof(lights)/sizeof(lights[0])); i++) //it would accomplish the same thing as long as we dont change type
Thanks for everyone's input.
Jeff

int nine [] = {13,12,11,5,4};
int eight []= {13,12,11,5,4,3,2};
int seven []= {13,12,11};
int six []= {13,11,5,4,3,2};
int five []= {13,11,5,4,2};
int four []= {13,12,5,4};
int three []= {13,12,11,4,2};
int two []= {12,11,4,3,2};
int one []= {13,12};
int zero []= {13,12,11,5,3,2};

void setup(){
  pinMode(13, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(2, OUTPUT);
}

void loop(){
  lightNum(nine);
  lightNum(eight);
  lightNum(seven);
  lightNum(six);
  lightNum(five);
  lightNum(four);
  lightNum(three);
  lightNum(two);
  lightNum(one);
  lightNum(zero);
}

void lightNum(int* lights){
  for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)
    digitalWrite(lights[i], HIGH);
  delay(1000);
  for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)
    digitalWrite(lights[i], LOW);
}

Well there are other ways ...

Further to my great interest in the STL (standard template library) here is how you could solve it with STL:

#include <iterator>
#include <vector>
#include <new.cpp>

using namespace std;

// find how many items in an array
#define NUMITEMS(ary) (sizeof (ary) / sizeof (ary [0]))

// our vectors
vector<byte> six;
vector<byte> seven;
vector<byte> eight;

// display the lights
void lightNum(vector<byte> & lights){
  
  for (vector<byte>::const_iterator i = lights.begin (); i != lights.end (); i++)
    digitalWrite(*i, HIGH);
  delay(1000);
  for (vector<byte>::const_iterator i = lights.begin (); i != lights.end (); i++)
    digitalWrite(*i, LOW);
}


void setup(){
  pinMode(13, OUTPUT);
  
  // method 1 of initializing a vector
  six.push_back (13);
  six.push_back (11);
  six.push_back (5);
  six.push_back (4);
  six.push_back (3);
  six.push_back (2);
  

  // method 2
  int seven_init []= { 13,10,11,5,4,3,2};
  copy (seven_init, &seven_init [ NUMITEMS (seven_init) ], back_inserter (seven));


  // method 3
  eight.resize (8);  // set size to 8
  eight [0] = 13;
  eight [1] = 12;
  eight [2] = 11;
  eight [3] = 10;
  eight [4] = 9;
  eight [5] = 6;
  eight [6] = 4;
  eight [7] = 2;
  
}

void loop(){
  lightNum(six);
  lightNum(seven);
  lightNum(eight);
}

The STL can be downloaded from here:

(197 Kb.)

Follow the instructions to install.

This may be somewhat of an overkill to light some LEDs' but I gather this is supposed to be educational, and STL is something that is very useful in programming in general.

One problem with STL vectors is that it isn't particularly easy to initialize them from a constant list. I show in the code above three ways you could go about doing it.

One of the nice things about the STL containers (including vectors, maps, sets etc.) is that you can find the size of them easily. I didn't actually need to in the code, because the iterator goes from the first to the last position, without you explicitly needing to know the size. But if you did:

int size = six.size ();

The STL vectors can be dynamically resized, which isn't possible with fixed arrays, and a pain with pointers.

for(int i=0; i<(sizeof(lights)/sizeof(int)); i++)

No. The size of the pointer is 2 (on this hardware), regardless of how many elements it points to. That won't work. If you want to use this method you have to take the sizeof the "real" array at the calling side (not the size of the pointer) and pass both the pointer and the size.

Like this:

// find how many items in an array
#define NUMITEMS(ary) (sizeof (ary) / sizeof (ary [0]))

void lightNum(int* lights, int n){
  for(int i=0; i<n; i++)
    digitalWrite(lights[i], HIGH);
  delay(1000);
  for(int i=0; i<n; i++)
    digitalWrite(lights[i], LOW);
}

void loop(){
  lightNum(nine, NUMITEMS (nine));
  lightNum(eight, NUMITEMS (eight));
  ...
}

The idea is to countdown on a 7 segment. They got it to work by sending the size of the array down each time it was called, but we are wondering where we went wrong, it seems to me that had we done

If it is a seven-segment display, why not make things easier, and pass an array of seven elements each time.
If you're worried about the amount of RAM this would use, reduce the size of the array by using byte types instead of ints.

A more usual way of doing this would be having one array of seven elements containing the pin numbers, and then single bytes for each of the numerals. The bits in the byte tell the subroutine which elements to light, something like this:

const byte LEDpins [7] = {13, 12, 11, 10, 6, 5, 4};
const byte digitCodes [10] = {B01111110,    // pattern for a zero  (for illustration only
                                        B00000110,    // pattern for a one  etc.

So, if bit zero is set, it indicates that the LED attached to LEDpins [ 0 ] is to be lit.

theborland:
The idea is to countdown on a 7 segment. They got it to work by sending the size of the array down each time it was called, but we are wondering where we went wrong, it seems to me that had we done:
for(int i=0; i<(sizeof(lights)/sizeof(lights[0])); i++) //it would accomplish the same thing as long as we dont change type

Take another look at reply #5 and the discussion that followed. The trouble is that your lightnum routine takes an int* parameter. That is just a pointer to some memory location. Lightnum only knows that it's been passed a pointer, it does not have any additional knowledge of what that pointer points to. Specifically, it doesn't know when you pass it six, that it's being passed a pointer to an array that is six elements long, so sizeof is not doing what you expect - it can't. Lightnum doesn't know whether you passed a pointer to an area of memory that you initialized with 10 elements or 100. You're asking it for sizeof(int*), which on the Arduino is 2. What you're expecting is sizeof(six), but lightnum has no way to know that, unless you pass in the size too.