« Posts tagged dumb coding mistakes

Dumb Coding Mistakes #632

Okay here’s a good one that had me stumped all afternoon.

I must say that as far as collections go, I’m used to using arrays. Arrays are great cuz they’re typed, they’re fast, and they’re relatively easy to set up and iterate through. The problem is they’re not dynamic. So it’s tough to quickly add and delete stuff from an array on the fly.

Enter the List. Lists are typed like arrays, but they are dynamic– meaning you can just add stuff to your List and the list grows. If you zap something from your list the list shrinks.

This is very useful for saving. In my app, users can add photos to use as backgrounds that they either take themselves or pick from their camera roll. Saving this data is great for a List. I don’t know how many different photos they will add over the course of the app’s life on their device and I don’t care. Save and delete your photos as you wish! They’ll all be stored in a List.

The dumb coding mistake is as follows.

In startup, I need to load these photos. So I iterate through the List. I also check to see if there are problems. If there is missing data or null data I simply throw up my hands and zap the thing from the List. Something went wrong, I’m destroying it. The user can import it again if need be. This isn’t like someone’s bank account.

So it goes something like this:


for (int i = 0; i < listOfBackgrounds.Count; i++) { if (listOfBackgrounds[i].IsScrewedUp()) { // list item is messed up or corrupted! // zap it! listOfBackgrounds.RemoveAt(i); } else { // everything is fine! load the background listOfBackgrounds[i].LoadBg(); } } Simple enough, right? We loop through all the items in the list. If we detect an error we zap the offending object and remove it from the list. If it's fine, we load it. ERROR! NULL REFERENCE EXCEPTION! Do you spot the mistake? It had my tearing my hair out for quite a while. Having been used to working with arrays instead of Lists, I overlooked an important quality of Lists. They are dynamic. Most of the time this is great. It allows you to add and drop stuff on the fly. But the problem with that, is the List is always CHANGING! Take the following array: myArray = new string[] {A, B, C, D}; It stores four strings. A is in index 0. B is in index 1. C is in index 2. D is in index 3. (both arrays and lists start counting their indexes at 0 -- itself the source of many dumb coding errors) If we DELETED the B at index 1, what would the value of index 3 be? Index 3 would still be D. Index 1 would have a null since we zapped the B. But I would still know what was in index 3 (and all the other indexes for that matter). A LIST on the other hand, continues to resort itself. So for a similar list with values of A,B,C,D in indexes 0,1,2,3... What would be the value at index 3 if we removed the value at index 1? It would be NULL! If we remove the value at index 1-- ie if we remove the B-- then the ENTIRE LIST changes to become: A,C,D in indexes of 0,1,2 Index 3 does not exist anymore! Everything shifts over. And thus my dumb coding mistake. I was iterating through a LIST while removing things from the list. So the list was CHANGING as I was trying to access it. The solution? Store the indexes I wanted to zap in a SEPARATE LIST which I then looped through AFTER the main for loop. After iterating through the main list and storing the numbers, only then was it safe to zap stuff from the original list and feel confident I knew what I was zapping. Even then, I had to loop through the separate list BACKWARDS. Removing index 5 wouldn't have any effect on indexes 0,1,2,3,4. So as long as I removed indexes from highest index to lowest index, even though the List was dynamically shifting, it wouldn't screw everything up. Another brainteaser for the books. I've learned that the only way through this stuff is banging your head against the keyboard until your stupidity gives up and leaves you alone.

Dumb Coding Mistakes #1

It’s easy to make dumb mistakes when you’re coding. Like here’s one I always make.

Say I have a slider for the interface with a knob in the middle you can slide back and forth. I need to check the position of the knob to see if it has moved left or right.

So I’m always tempted to do:

void CheckSliderPos() {
if (myKnob.transform.localposition.x > 5f) {
// do some stuff cuz the knob x pos is greater than 5 now
} else if (myKnob.transform.localposition.x < 3f) { // do some stuff cuz the knob x pos is less than 3 now } }

So what's wrong with this? Usually, NOTHING! And that's the problem. Usually this sort of check is just fine. But what if there are more things to check? What if there is a whole list of if thens. And then what if the user slides the knob REALLY FAST?

Well, in this case there is the rare possibility that the user SLIDES THE SLIDER faster than the machine can compute the code. And this leads to ugly bugs.

So imagine this:

void CheckSliderPos() {
if (myKnob.transform.localposition.x > 5f) {
// do some stuff cuz the knob x pos is greater than 5 now

// } else if { (etc)
// } else if { (etc)
// } else if { (etc)

// Do you see the problem?

// PROBLEM!!!!
// BY THIS POINT, the knob position may very likely have changed significantly from
// where it was when we checked at the top.
} else if (myKnob.transform.localposition.x < 3f) { // do some stuff cuz the knob x pos is less than 3 now } }

This can quickly lead to whacky results.

The proper way to do it, of course, is to STORE the knob position immediately as a constant... and use the same constant to check against all the logic.

Like so:

void CheckSliderPos() {

// STORE THE POSITION so we can compare against a CONSTANT position
float knobXPos = myKnob.transform.localposition.x;

// THEN check your logic and you'll be fine.
if (knobXPos > 5f) {
// etc
} else if (knobXPos < 3f) { // etc } }

This is such an easy and basic mistake, but I find myself making it all the time. A great example, I think, of how there are little tricky pitfalls in coding that can surprise you if you're not carefully thinking everything through.