2
\$\begingroup\$

The textbook problem:

(strictly identical arrays) Two arrays list1[] and list2[] are strictly identical if they have the same length and list1[] is equal to list2[] for each[i].

Write a function that returns true if list1 and list2 are strictly identical using the following header:

bool strictlyEqual(const int list1[], const int list2[], int size)

Write a test program that prompts the user to enter two lists of integers and dis­plays whether the two are strictly identical. The sample runs follow. Note that the first number in the input indicates the number of the elements in the list. This number is not part of the list. Assume the list size is maximum.

Any general tips to clean up this code? It just seems that I can accomplish the task w/o doing all the checks.

   #include <iostream>
   using namespace std;

   bool strictlyEqual(int const list1[], int const list2[], int size);

   bool strictlyEqual (int x1[], int x2[], int n)
   {
     int f=0; int i;
     for (i =1; i<=n; i++)
     {
               if (x1[i] != x2[i])
               {
                  // breaks loop
                    f=1;
                    break;
                }
     } 

 if (f==0)
 return (true);
 else 
 return(false);
 }


int main () {
cout << "enter list1: " << endl;
int list1[20], i;
cin >> list1[0];
for (i=1; i<= list1[0]; i++)
cin>> list1[i];

cout <<"enter the list2" << endl;
int list2[20];
cin >> list2[0];
for (i=1; i<= list2[0]; i++)
cin >> list2[i];

if (list1[0] == list2[0])
{
     int size = list2[0];
     bool v=strictlyEqual(list1, list2, size);
     if (v== true)
     cout << "identical" << endl;
     else 
     cout << "not identical " << endl;
 }

  return 0;
 }
\$\endgroup\$
2
  • \$\begingroup\$ Does this code even compile? It doesn't look like it does. "Note that the first number in the input indicates the number of the elements in the list. This number is not part of the list." Also, the code fails this requirement, as it passes the size as an element of the list. \$\endgroup\$
    – mdfst13
    Dec 5, 2016 at 7:53
  • \$\begingroup\$ if the function signature is from a textbook, you should return it. Try to use either this or this book, since they have much higher review on SO. \$\endgroup\$ Dec 5, 2016 at 12:10

2 Answers 2

1
\$\begingroup\$

I see a number of things that may help you improve your code.

Fix your formatting

The code as posted has inconsistent indenting which makes it hard to read. Choose a particular style and apply it consistently to make your code easier to read.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Validate the input

Nothing in the code currently prevents the user from entering a list size of 3000 or -98. The main routine should validate the input size and reject numbers that are not within range.

Don't use std::endl if you don't really need it

The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream. This can be time-consuming in a program with a lot of I/O and is rarely actually needed. It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one. Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.

Simplify boolean expressions

The code currently contains this:

if (f == 0)
    return (true);
else
    return (false);

However, since the == operator already returns a bool, you can simplify this to a single line:

return f == 0;

Also note that return is not a function and does not need parentheses. Similarly within main, we have this:

bool v = strictlyEqual(list1, list2, size);
if (v == true)
    cout << "identical" << endl;
else
    cout << "not identical " << endl;

But one could instead write this:

if (strictlyEqual(list1, list2, size)) {
    std::cout << "identical\n";
} else {
    std::cout << "not identical\n";
}

Or perhaps this:

std::out << (strictlyEqual(list1, list2, size) ?  "" : "not ") 
        << "identical\n";

Read the problem statement carefully

The problem says that the size is not part of the array, but your program inserts the size as the first element of the array. Instead, you could read the size from the user and store it in a separate variable that is passed to the strictlyEqual function. Also note that as the current code is written, if the two lists are not of equal size, the program prints nothing.

Make sure your arguments match

The declaration of the function in the current code is this:

bool strictlyEqual(int const list1[], int const list2[], int size);

but the actual defintion has this:

bool strictlyEqual(int x1[], int x2[], int n)

First, if the function is defined before use, there's no need for a separate declaration at all. Second, if you do have a separate declaration, make sure that everything matches, including const and the parameter names. Strictly speaking, it's not an error to have different parameter names, but it can be confusing to readers of the code, so it's generally best to have them match. A difference in const, however, modifies the signature of the function

Think more carefully about the algorithm

Because only one size is passed to the function, it seems that we must assume that the two arrays are of equal size. (I'd say this is a flaw in the question, but presumably out of your control.) Given that, the function only returns true if all elements are equal, so the simple way to do that would be this:

bool strictlyEqual(const int list1[], const int list2[], int n) {
    for (int i=0; i < n; ++i) {
        if (list1[i] != list2[i]) {
            return false;  
        }
    }
    return true;  // we only get here if every element was equal
}

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.

\$\endgroup\$
0
\$\begingroup\$

Shorter:

bool strictlyEqual (int x1[], int x2[], int n)
{
    while (n-- > 0)
    {
        if (*x1++ != *x2++)
        {
            return false;
        }
    }
    return true;
}

A statically-sized arrays are more C-like than C++, though, and I think you were supposed to use some vector class. Then it makes sense to request comparing lengths of both arrays ('if they have the same length')...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.