working with pset3

Question

why isnt the compiler returning the false if the needle is not in the haystack.This is my modified code with the SESE method.

/**
 * helpers.c
 *
 * Computer Science 50
 * Problem Set 3
 *
 * Helper functions for Problem Set 3.
 */

#include <cs50.h>

#include "helpers.h"

/**
 * Returns true if value is in array of n values, else false.
 */
bool search(int value, int values[], int n)
{
    // TODO: implement a searching algorithm
    int i, r;

    for(i = 0; i < n; i++)
    {
        int value = 0;
        int values[i];
        if(values[i] == value)
        {
            r = 1;
        }
        else
        {
            r = 0;
        }
    }
    return r;
}

/**
 * Sorts array of n values.
 */
void sort(int values[], int n)
{
    // TODO: implement an O(n^2) sorting algorithm
    return;
}

but it dosent return the false value.

Answer 1

Unless the code is searching for a 0, it will never return true. You've created something called a shadow variable value inside the for loop and set it to 0, so it will always search for 0.

A shadow variable is created when there is already a variable with a particular name, and a new variable is created inside a set of curly braces with the same name. The original variable is hidden and overridden by the new shadow variable which has local scope. When the code moves outside the curly braces that define the scope of the shadow variable, it will cease to exist and the original variable will be exposed and available again.

In this case, the original value var is one of the parameters of the function. Then, a shadow var of value is created inside the for loop. The curly braces of the for loop define the scope of the shadow variable.

Even if a shadow var hadn't been declared, assigning 0 to value will cause the actual needle to be lost - replaced with 0. The impact of that should be obvious.

After that, what happens if the needle you're looking for isn't the last number in the list? Say it's next to the end. The code will set r = 1 when the needle is found, but will then set it back to 0 when the next number is checked. Only when the needle is last will it even think about returning true.

The simple solution is to return true immediately when the needle is found to short-circuit processing, or to just keep processing, and return false if not found.

As a side note, when returning or working with a bool, it is far better to use true and false instead of numbers. While a 0 universally means false, different library functions use different numbers for true. Any non-zero is considered true, but what happens when two different programmers use different numbers for true in the same program? Or, don't account for what different functions return for true? (Historically, I believe it goes back to a bit mask routine used by registers.) It also eliminates any likelihood of confusion.

There may be other issues, but this is far and away the biggest problem. If you find any other issues, time for a new question.

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)