1

I wrote a program that examines whether a number in a sequence is repeated 2 times (so many times it should be repeated) if it is repeated 2 times "YES", if not "NO". I think this can be written in a simpler and better way, so I'm interested in the suggestions and corrections of someone more experienced than me.
The program should print YES if each member of array is repeated exactly once, and otherwise it should print NO:

Enter the number of array: 8
Enter the sequence: 1 2 2 1 3 3 4 4
YES
Enter the number of string members: 7
Enter the string: 1 2 1 2 1 3 4
NO

Here is my code:

#include <stdio.h>

#define arr 100

int main() {
    int n, i, p = 1, j, a;
    double array[arr];
    int num[100];
    do {
        printf("Enter the number of array: ");
        scanf("%d", &n);
    } while (n < 1 || n > 100);

    printf("Enter array: ");
    for (i = 0; i < n; i++) {
        scanf("%lf", &array[i]);
        num[i] = -1;
    }
    for (i = 0; i < n; i++) {
        a = 1;
        for (j = i + 1; j < n; j++) {
            if (array[i] == array[j]) {
                a++;
                num[j] = 0;
            }
        }
        if (num[i] != 0)
            num[i] = a;
    }

    for (i = 0; i < n; i++) {
        if (num[i] == 0)
            continue;
        if (num[i] != 2) {
            p = 0;
            break;
        }
    }
    if (p == 1)
        printf("YES");
    else
        printf("NO");
    return 0;
}
Improve this question
5
  • 2
    Your program prints "NO" for the sequence of 4 numbers 1 2 2 3. Is that correct? Commented Jul 26, 2022 at 12:49
  • 2
    @Bob__ if the code prints "NO" for the sequence of the 4 numbers 1 2 2 3 then the code isn't working and the question doesn't belong on Code Review. C code that has no indentation probably doesn't belong on code review. Commented Jul 26, 2022 at 15:10
  • @Bob__ The program should print YES if each member of array is repeated exactly once (in total occurs twice in array), and otherwise it should print NO. Commented Jul 26, 2022 at 15:15
  • You can learn how to properly format the C programming language using codebeautify.org/c-formatter-beautifier Commented Jul 26, 2022 at 15:17
  • 1
    You only need memory of the last entry, then the entry is not used ever again. This suggests it would be better to take values that are updated on-line instead of an arbitrary limit (100). Commented Jul 26, 2022 at 17:59

2 Answers 2

Reset to default
3

Your code works, but there are some issues:

  • you should test for conversion failures in scanf(): the return value must be 1 as there is a single conversion for each scanf() call.

  • the counting loops are too complicated: just counting the number of occurrences with 2 nested loops is much simpler and has comparable time complexity, and using a += (array[i] == array[j]); may produce fast branchless code on most architectures.

  • it is good style to finish the program output with a newline.

Here is a modified version:

#include <stdio.h>

int main() {
    int n;

    do {
        printf("Enter the length of the array: ");
        if (scanf("%d", &n) != 1)
            return 1;
    } while (n < 1 || n > 100);

    double array[n];

    printf("Enter array: ");
    for (int i = 0; i < n; i++) {
        if (scanf("%lf", &array[i]) != 1)
            return 1;
    }
    int p = 1;
    for (int i = 0; i < n; i++) {
        int a = 0;
        for (int j = 0; j < n; j++) {
            a += (array[i] == array[j]);
        }
        if (a != 2) {
            p = 0;
            break;
        }
    }
    if (p == 1)
        printf("YES\n");
    else
        printf("NO\n");
    return 0;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

3

I am going to provide a code review here because the code doesn't work and that makes it off-topic on Code Review.

Good things about the code:

  • There are no global variables.
  • There is an attempt to define a symbolic constant for the array size.

Things that can be improved:

  • Declare the variables are you need them. In the original version of C back in the 1970s and 1980s variables had to be declared at the top of the function. That is no longer the case, and a recommended programming practice to declare the variable as needed. In C the language doesn't provide a default initialization of the variable so variables should be initialized as part of the declaration. For readability and maintainability each variable should be declared and initialized on its own line.
  • Capitalize the constants, arr should be ARR.
  • ARR should be used in the declaration of num as well as the declaration of array.
  • ARR should be used instead of 100 in this statement while (n < 1 || n > 100).
  • Since the program only allows a single digit to be read at a time, array should be an array of integers rather than array of doubles.
  • There is only one error check on user input.

A simplified version of the program that does work:

One of the first things you need to learn in programming is how to write functions and subroutines. This is important because it allows you to break problems into smaller and smaller pieces until it is very easy to solve the problem. There are a number of programming principles that this relates to, 2 of them are the Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

and the KISS Principle (Keep It Simple).

#include <stdio.h>
#include <stdbool.h>

#define ARR 100
static int get_user_input(int array[ARR])
{
    int count = 0;

    do {
        printf("Enter the number of array: ");
        scanf("%d", &count);
    } while (count < 1 || count > ARR);

    printf("Enter array: ");
    for (int i = 0; i < count; i++) {
        scanf("%1d", &array[i]);
    }

    return count;
}

static bool find_first_repetition(int count, int array[ARR])
{
    bool repetition = false;
    
    for (int i = 1; i < count; i++)
    {
        if (array[i - 1] == array[i])
        {
            return true;
        }
    }

    return repetition;
}

int main() {
    int n;
    int num[ARR];

    n = get_user_input(num);

    if (find_first_repetition(n, num))
    {
        printf("YES");
    }
    else
    {
        printf("NO");
    }

    return 0;
}
Improve this answer

7 Comments

The first C to allow intermixing of declarations was C99, it was released in 1999, and adopted in 2000. I just checked and clang and gcc show warnings when the standard is set to GNU90, so I think the 1990s should be on your dates.
Please note the OP's problem statement: "The program should print YES if each member of array is repeated exactly once (in total occurs twice in array), and otherwise it should print NO." and the results of both program: OP's vs yours.
@Neil The OP cross posted this question on Code Review. Their comments there are that they are a beginner in C looking to learn better ways, unfortunately the question was off-topic there so I answered the question here giving advice I would normally give on Code Review. You are correct, this won't compile prior to C99 but I didn't feel it was necessary to confuse the OP given that they are new to C. I started programming in C in 1983, this won't even come close to compiling in K&R.
@Bob__ I generally only answer posts on Code Review and it is possible my answer doesn't follow all the Stack Overflow guidelines. My goal was to give the OP some guidelines on how to code C better and not necessarily debug their code. This was based on their comments on their other post (link above in another comment). I would not have answered this question if you hadn't directed the OP to Code Review.
Well, in that comment (that got deleted) I clearly stated that a program should be working correctly to be accepted there. I also added a link to the tour, but I accept the blame. It bugs me, though, that you guys keep saying (and chose as close reason) that OP's program doesn't work as intended. It does.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.