I am trying to return true if the two arrays have common values and return False otherwise

Question

I am trying to return true if the two arrays have common values and return False otherwise. the problem is when running this code I found that arrays are not as declared. one of the two arrays contains the values of both arrays

Here is the Code:

#include<bits/stdc++.h>
using namespace std;

bool commonValues(char arr1[], char arr2[]){

    for (int i = 0; i < strlen(arr1); i++){
        for(int j = 0; j < strlen(arr2); j++){
            if (arr1[i] == arr2[j]){
                return true;
            }
        }

    }
    return false;

}

int main(){
    char arr1[] = {'a', 'b', 'c', 'd', 'z', 'x', 'k', 'l'};
    char arr2[] = {'e', 'f', 'g', 'h'};
    for (int i = 0; i < strlen(arr2); i++){
    }

    cout<<commonValues(arr1, arr2)<<endl;

    return 0;
}

`for (int i = 0; i < strlen(arr1); i++){for(int j = 0; j < strlen(arr2); j++){` -- Off-topic, but this is most naive way to accomplish your goal. Imagine if the string(s) had thousands of elements -- how many loop iterations would that be? The second issue is that you're calling `strlen` on every iteration. — PaulMcKenzie, Dec 13 '21 at 16:11
You should add null terminator for the array, e.g. `char arr1[] = {'a', 'b', 'c', 'd', 'z', 'x', 'k', 'l', '\0'};`, `char arr2[] = {'e', 'f', 'g', 'h', '\0'};`. — songyuanyao, Dec 13 '21 at 16:11
`strlen` only works with NUL terminated `const char*` arrays. Use `std::size`. — Bathsheba, Dec 13 '21 at 16:14
@PaulMcKenzie I am aware of that. Actually I am studying a data Structure and Algorithm course right now. I was trying to code the Brute force solution of the Problem. Also, I do not think strlen() is being called every iteration, you do ? — Muhammad Elmallah, Dec 13 '21 at 16:18
@MuhammadElmallah You still have the issue of calling `strlen` on each iteration. Call it once, save the value, and use that saved value in the `for` loop. If you are not aware, `strlen` has to count the number of characters until a `null` is reached. It doesn't automatically know the number of characters -- it needs to count. — PaulMcKenzie, Dec 13 '21 at 16:19
@PaulMcKenzie: Or run the loops backward. Not for the faint-hearted though - unsigned arithmetic and therefore possible use of `--> 0`. — Bathsheba, Dec 13 '21 at 16:20
@songyuanyao Yes. this is right, Thank you for this. But if I am given 2 arrays from the user, are not they supposed to have their 'null' ? — Muhammad Elmallah, Dec 13 '21 at 16:24
@Bathsheba This Gives me an error and suggest I use 'dsize' instead. do you have any idea? — Muhammad Elmallah, Dec 13 '21 at 16:27
@MuhammadElmallah -- `#include` -- Get rid of this and use the proper headers, `` and ``. You also claimed you are taking a class -- no C++ teacher or class should have given you this header. If they did, you're not learning C++ properly. Possibly the `dsize` error you're getting is related to using this header. — PaulMcKenzie, Dec 13 '21 at 16:29
Aaargh don't use char arrays (even worse than pointers) for strings in C++! Also please don't include anything from `bits` , it's not supposed to be included by consumers of the standard library! — andreee, Dec 13 '21 at 16:51

andreee · Answer 1 · 2021-12-13T17:18:57.083

Here is a modern C++ variant that will work for you:

#include <iostream>
#include <string_view>
#include <unordered_set>

using namespace std::string_view_literals;

bool commonValues(std::string_view a, std::string_view b) {
    std::unordered_set<char> chars_in_a;
    for (auto c : a) {
        chars_in_a.insert(c);
    }
    
    for (auto c : b) {
        if (chars_in_a.contains(c)) {
            return true;
        }
    }
    return false;
}

int main() {
    constexpr auto str_a = "abcdzxkl"sv;
    constexpr auto str_b = "efgh"sv;
    constexpr auto str_c = "jptxo"sv;

    std::cout << str_a << " contains at least one character of " << str_b << ": ";
    std::cout << std::boolalpha << commonValues(str_a, str_b) << std::endl;

    std::cout << str_a << " contains at least one character of " << str_c << ": ";
    std::cout << std::boolalpha << commonValues(str_a, str_c) << std::endl;
}

Advantages:

Uses strings/string views rather than arrays and pointers. Avoid using pointers and raw arrays in C++ unless you need to (e.g. when performing some very low level operations). std::string is usually the way to go, and if you are merely inspecting a string (like in your function commonValues), you'd usually want to pass a string_view for efficiency. In your case the string never changes, so you can even initialize with a constexpr string_view.
Runs in O(n) rather than O(n*m) (where n is the longer string length). Instead of having a nested loop over both arrays, we simply store what we've "seen" so far into a (unordered) set, then we can simply query in O(1) (avg.) whether an element of the second string is included in the first one. We are sacrificing O(n) of memory here, but usually you'll be hitting the runtime barrier way sooner in such algorithms.

Please add a comment in case further clarification is required.

Jason · Answer 2 · 2021-12-13T17:26:47.293

One way to do this would be by using template non-type parameters as shown below:

Version 1: Using templates

#include<iostream>

//arr1 and arr2 are reference to array of size M and N respectively
template<std::size_t M, std::size_t N>
bool commonValues(char (&arr1)[M], char (&arr2)[N]){

    for (int i = 0; i < M; i++){//M USED HERE
        for(int j = 0; j < N; j++){//N USED HERE
            if (arr1[i] == arr2[j]){
                std::cout<<"common element found: "<<arr1[i]<<std::endl;
                return true;
            }
        }
    }
    return false;

}

int main(){
    char arr1[] = {'a', 'b', 'c', 'd', 'z', 'x', 'k', 'l'};
    char arr2[] = {'e', 'f', 'g', 'h'};

    std::cout<<commonValues(arr1, arr2)<<std::endl;

    return 0;
}

Another way would be to pass the size of the arrays to the function as shown below.

Version 2: Passing the size of array as arguments

#include<iostream>
#include <string> //needed for std::size with C++17
//the 2nd and the 4th parameters are the size of the arrays
bool commonValues(char *arr1, std::size_t M, char *arr2, std::size_t N){

    for (int i = 0; i < M; i++){//M USED HERE
        for(int j = 0; j < N; j++){//N USED HERE
            if (arr1[i] == arr2[j]){
                std::cout<<"common element found: "<<arr1[i]<<std::endl;
                return true;
            }
        }
    }
    return false;

}

int main(){
    char arr1[] = {'a', 'b', 'c', 'd', 'z', 'x', 'k', 'l'};
    char arr2[] = {'e', 'f', 'g', 'h'};
    //note that we are passing 4 arguments this time.
    std::cout<<commonValues(arr1, std::size(arr1), arr2, std::size(arr2))<<std::endl;

    return 0;
}

I am trying to return true if the two arrays have common values and return False otherwise

2 Answers2