Remove unneeded variables and conditions

[dcbriccetti]

I’ve recently discovered these helpful examples. I’d like to offer some code simplifications.

Accelerometer

If I’m not mistaken, x, y, and z are never (or extremely rarely) 0, so the “if”s are not needed, and then the R, G, B variables are not needed. Did I miss something?

Buttons

I find my new Python students are comfortable with conditional expressions, but if the
consensus here is that the if statement is preferred, I can remove that commit.

[tannewt]

I do think my preference is to leave the if statements as is. The expression version is yet another form someone has to learn.

For the RGB case, is there any time x, y or z is something that can't be converted to an int? The if checks may be guarding against bad data.

[dcbriccetti]

Hi Scott. Thanks for having a look.

I removed the if expression commit from the branch.

About the 9 lines of code (with three “if”s) to possibly ignore unreasonable values, how likely is that? Perhaps I’ll go follow the code and see what cpx.acceleration can return.

>>> type(_lis3dh.acceleration)
<class 'acceleration'>

I’m looking for this now. (This is a fascinating learning experience!)

I think it’s here. I see nothing indicating this returns anything other than a tuple of floats. Please educate me if I’m wrong.

[dcbriccetti]

You know, I probably should have started by asking @kattni the reason for the “if”s.

[tannewt]

I can't remember why they are there. Hopefully she does. If not, we can always try it and see if it breaks. :-)

[dcbriccetti]

If the ifs are needed, an explanatory comment might be in order.

[kattni]

I think the idea was to be able to set a baseline value if you wanted for R, G and B.

[dcbriccetti]

Thanks, @kattni.

What’s best to do here? Would you prefer to leave the file as it is?

[kattni]

Is it worth maintaining that functionality by retaining the R = 0 etc. and then changing this line to:

cpx.pixels.fill(((R + abs(int(x))), (G + abs(int(y))), (B + abs(int(z)))))

Turns into a lot of parens, but allows for increasing the overall brightness of things by allowing increased baseline RGB values.

Or does setting R = x + 0 and then making it cpx.pixels.fill((abs(int(R)), ... work? That would simplify the fill statement, and also might make it easier to see the RGB/xyz relationship.

I haven't tested either of those, I'm simply suggesting here.

@dcbriccetti What do you think?

[dcbriccetti]

I think there’s value in explicitly showing the mapping between acceleration components and RGB components.

[kattni]

I don't entirely understand what you've done here. If the intention was to simplify the code, I feel that we've moved away from the initial intention. You've introduced a significant number of new concepts. These examples, for the most part, are meant to be simple demonstrations of using the Circuit Playground Express library.

If you'd like to, I would suggest submitting your current iteration as an entirely new example. I don't want to entirely do away with the simple version which updating to this would do.

[kattni]

We typically include defs before the loop at the top of the code below the imports. Consider moving this.

[dcbriccetti]

Where’s the embarrassed emoji? :-)

Yes, I sensed this was going the wrong direction.

[dcbriccetti]

retaining the R = 0 etc. ...

OK, I like that idea.

[kattni]

Looks great! Thanks for sticking with us through this!

[dcbriccetti]

Glad to be working with you to help with education.

Do you think there’s much value to the Adafruit audience of my more complicated version? I can always just use it with my students, and maybe make a YouTube video explaining it.

[kattni]

@dcbriccetti I think there's value to having multiple options. I would suggest explaining it thoroughly though, as it was unclear to me, so I imagine it will be unclear to many. If you make a video explaining it, you could include a link in a comment.

[dcbriccetti]

The video is here, https://youtu.be/eNpPLbYx-iA, and I’ll make a new pull request for the new program.