Roku Developer Program

[dan19]

sub addToList()

  matched = false
  countIndex = 0
  if m.global.watchList.Count() > 0
    m.movieArray = m.global.watchList
    for each item in m.global.watchList
      if item.TITLE = m.selectedMovie.TITLE
        m.movieArray.delete(countIndex)
        m.global.watchList = m.movieArray

        matched = true
      else
        m.movieArray.push(m.selectedMovie)
        m.global.watchList = m.movieArray
        matched = false
      end if
      if matched then  EXIT FOR
      countIndex = countIndex + 1
      
    end for
  else
    m.movieArray.push(m.selectedMovie)
    m.global.watchList = m.movieArray
  end if

end sub

I am practicing and simulating an actual watchlist array. The issue with the code here is; after adding two movies to the watchList (which works fine), when I add a third one, it adds it twice, and when I add a fourth one, it adds it four times, and so on.

What am I doing wrong with the logic?

Thanks.

[renojim · Accepted solution]

Mixing "for each" and indices is a bad idea, but that's only part of the problem.  You're going to be pushing something every time the title doesn't match as you go through the loop.  If I'm following what you're trying to do you should do something like:

for i = 0 to m.global.watchList.Count() - 1
   if m.global.watchList[i].title = m.selectedMovie.TITLE then
      m.global.watchList.delete(i)
      exit for
   end if
end for
if i = m.global.watchList.Count() then m.global.watchList.Push(m.selectedMovie)

You could also use a matched flag, but if i is past the end of the watchList at the end of the for loop then it wasn't matched.

A note about your code:  using m.movieArray isn't really accomplishing anything.  Setting it equal to m.global.watchList doesn't create a copy of it, just another reference to the same data.  Take this example:

x = [1,2,3]
y = x
y.delete(1)
print x
print y

It will give:

<Component: roArray> =
[
    1
    3
]
<Component: roArray> =
[
    1
    3
]

[dan19]

---

@renojim wrote:

Mixing "for each" and indices is a bad idea, but that's only part of the problem.  You're going to be pushing something every time the title doesn't match as you go through the loop.  If I'm following what you're trying to do you should do something like:

for i = 0 to m.global.watchList.Count() - 1
   if m.global.watchList[i].title = m.selectedMovie.TITLE then
      m.global.watchList.delete(i)
      exit for
   end if
end for
if i = m.global.watchList.Count() then m.global.watchList.Push(m.selectedMovie)

You could also use a matched flag, but if i is past the end of the watchList at the end of the for loop then it wasn't matched.

A note about your code:  using m.movieArray isn't really accomplishing anything.  Setting it equal to m.global.watchList doesn't create a copy of it, just another reference to the same data.  Take this example:

x = [1,2,3]
y = x
y.delete(1)
print x
print y

It will give:

<Component: roArray> =
[
    1
    3
]
<Component: roArray> =
[
    1
    3
]

---

Thanks for the explanation. I have tried pushing directly to global variables, but for some reason, it doesn't work. I never thought about the for loop style. I am still trying to figure out what and what I can do with brightscript. And thanks for pointing out the issue with assigning the value of the global array to the movieArray. I'll check this out and get back to you.

P.S: for the line coloured in your code, will "i" work, since it is outside the for loop? I see it was initialized with the for loop.

Thanks a lot!

[renojim]

I don't know why pushing on a global variable wouldn't work.  Perhaps there's something else going on.

i will have a value outside the for loop.  It will either be the value it had when the loop hit the exit statement or one past the end of the loop (e.g., for i = 1 to 3 : end for, i will be 4 after executing this nothing loop). i doesn't go out of scope.

When it comes to BrightScript, print is your best friend and it's easy to get tripped up with almost everything being a reference.

[dan19]

---

@renojim wrote:

I don't know why pushing on a global variable wouldn't work.  Perhaps there's something else going on.

i will have a value outside the for loop.  It will either be the value it had when the loop hit the exit statement or one past the end of the loop (e.g., for i = 1 to 3 : end for, i will be 4 after executing this nothing loop). i doesn't go out of scope.

When it comes to BrightScript, print is your best friend and it's easy to get tripped up with almost everything being a reference.

---

Thanks buddy. I'll give this a bunch of tries, and get back to you.

[sanity-check]

  ''' add a field
  m.global.addField("test", "array", false)
  m.global.test = ["foo", "bar"]
  ?formatJson(m.global.test) ''' ["foo","bar"]
  ''' did it work? yep

  ''' can we add an element like this?
  m.global.test.push("zzz")
  ?formatJson(m.global.test) ''' ["foo","bar"]
  ''' nope... why not?

  ''' what about this?
  m.global.test[0] = "zzz"
  ?formatJson(m.global.test) ''' ["foo","bar"]
  ''' still no

  ''' let's pull it into a local var
  temp = m.global.test 
  temp.push("zzz")
  ?formatJson(temp) ''' ["foo","bar","zzz"]
  ''' ok, great - not going mad.. so this should have a zzz in..
  ?formatJson(m.global.test) ''' ["foo","bar"]
  ''' ...but it doesn't

  ''' Soo.. whenever we access m.global.test it's returning
  ''' a *copy* not a reference. and changing the copy does not
  ''' change m.global.test
  m.global.test = temp
  ?formatJson(m.global.test) ''' ["foo","bar","zzz"]
  ''' great, we finally manage to change our array! but was that
  ''' assignment a copy or a ref?
  temp.push("!!!")
  ?formatJson(m.global.test) ''' ["foo","bar","zzz"]
  ''' no "!!!" - so assigning is a copy too

  ''' what about nested references? let's find out
  temp.push(["a","b"])
  m.global.test = temp
  ?formatJson(m.global.test) ''' ["foo","bar","zzz","!!!",["a","b"]]
  temp[4][0] = "hi"
  ?formatJson(temp) ''' ["foo","bar","zzz","!!!",["hi","b"]]
  ?formatJson(m.global.test) ''' ["foo","bar","zzz","!!!",["a","b"]]
  ''' no references kept there either - so that assignment is
  ''' doing a *deep* copy

  ''' clearly that's going to get expensive with big objects, what
  ''' can we do instead?
  temp2 = createObject("roSgNode", "node")
  m.global.addField("test2", "node", false)
  m.global.test2 = temp2
  foo = createObject("roSgNode", "node")
  foo.id = "foo"
  bar = createObject("roSgNode", "node")
  bar.id = "bar"
  temp2.appendChild(foo)
  temp2.appendChild(bar)
  ''' we've added children to temp2 *after* assigning it  
  ''' to m.global.test2.. did they appear?
  ?m.global.test2.getChild(0).id ''' foo
  ?m.global.test2.getChild(1).id ''' bar
  ''' ...yes! so if we use scenegraph nodes we can actually
  ''' deal with references instead of values
  xxx = createObject("roSgNode", "node")
  xxx.id = "xxx"
  m.global.test2.appendChild(xxx)
  ?m.global.test2.getChild(2).id ''' xxx
  ''' great - it works directly too. 
  ''' it's just a shame it's a node, not a string!