0

why does this program run only till the first for loop and then stops?

It doesn't run the second for loop and also skips system("pause"). Can anyone explain what is wrong in my code? I want to make two arrays of strings: strgs1 and strgs2 of length a and b, and then take the input from the user for each element of the arrays. This is my code:

#include <cstdio>
#include <iostream>
#include <string>
using namespace std;

int main(){
    int a,b;
    cin>>a>>b;
    string strgs1[a-1], strgs2[b-1];
    
    for(int i = 0;i < a;i++){
        cin>>strgs1[i];
    }        
    for(int j = 0;j < b;j++){
        cin>>strgs2[j];
    }
    system("pause");
    return 0;
}
Community
  • 1
  • 1
Manik
  • 573
  • 1
  • 9
  • 28
  • 2
    `string strgs1[a-1]` Not to mention, that VLAs are non-standard C++, the last index your loops try to access is `a-1`, which is out of bounds of the array. – Algirdas Preidžius Nov 11 '18 at 15:10
  • 2
    Your program exhibits undefined behavior, by way of accessing an index out of bounds. `strgs1[a-1]` has `a-1` elements numbered `0` through `a-2`. The loop attempts to access an element at index `a-1` - there's no such element. – Igor Tandetnik Nov 11 '18 at 15:11
  • @AlgirdasPreidžius why is strgs[a-1] out of bounds?? the for loops condition is i < a so it will iterate till i = a-1. – Manik Nov 11 '18 at 15:14
  • @firangi *why does this program run* -- That program should fail to compile, let alone run. `string strgs1[a-1], strgs2[b-1];` -- That is not C++. – PaulMcKenzie Nov 11 '18 at 15:15
  • A program with UB may do *anything*. Don't write UB. – Jesper Juhl Nov 11 '18 at 15:16
  • @firangi For laughs, enter a negative number for `a` and `b`. – PaulMcKenzie Nov 11 '18 at 15:17
  • @PaulMcKenzie my minGW compiler g++ gives no error for this program – Manik Nov 11 '18 at 15:17
  • @firangi -- It isn't C++. That compiler is using VLA's (Variable Length Arrays), which is an *extension*, but isn't real C++. If you compiled with different compiler options, you will see your code will no longer compile. Stop using it and use `std::vector` instead. – PaulMcKenzie Nov 11 '18 at 15:18
  • @PaulMcKenzie you mean I should not use minGW for C++? `std::vector` is just a datatype no?. and btw the program is compiling. – Manik Nov 11 '18 at 15:21
  • @firangi mingW uses `g++`. The `g++` compiler *by default* allows such syntax. See [this post](https://stackoverflow.com/questions/31710642/disable-variable-length-automatic-arrays-in-gcc) to turn off this option. This is one complaint I have with `g++` -- it lets new programmers believe they are writing valid C++ with this VLA stuff. I wish the people that maintain that compiler would just turn VLA's *off* by default (and let all programmers know that version (whatever) will no longer, by default, support VLA's). – PaulMcKenzie Nov 11 '18 at 15:26
  • @firangi "_why is strgs[a-1] out of bounds??_" The last element of `strgs` has index of `a-2`. – Algirdas Preidžius Nov 11 '18 at 15:26
  • 1
    @firangi "my minGW compiler g++ gives no error for this program" - Compilers are *not* required to emit warnings or errors for undefined behaviour (or many other forms of broken programs). You *cannot* rely on "it compiles" as a test for "valid program". – Jesper Juhl Nov 11 '18 at 15:27
  • the compiler just skips everything after the for loop. so what is the right way to do this? @JesperJuhl I am not using a debugger. – Manik Nov 11 '18 at 15:28
  • 1
    @firangi As some of us (including the answers) already stated: You are iterating out-of-bounds of your array. That is undefined behavior. – Algirdas Preidžius Nov 11 '18 at 15:30
  • @firangi where did I mention a debugger? – Jesper Juhl Nov 11 '18 at 15:32
  • In case you did not catch it earlier; "undefined behaviour" means "bug in your program" where the compiler is allowed to do *whatever it pleases*. So *any* behaviour you observe is allowed. The solution is to fix your broken code to *not* contain UB. – Jesper Juhl Nov 11 '18 at 15:34
  • @AlgirdasPreidžius How is it out-of-bounds it will iterate till i = a-1 hence it will reach strgs1[a-1]. do you also mean VLA? – Manik Nov 11 '18 at 15:36
  • @Jesper Juhl you didn't say debugger but isn't that what a debugger does? just tell me the right code someone... – Manik Nov 11 '18 at 15:37
  • @firangi [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – Jesper Juhl Nov 11 '18 at 15:38
  • @JesperJuhl my program isn't running as I expected it to. How would you write this program? – Manik Nov 11 '18 at 15:41
  • I get that. Which is why you ought to a) learn the language. b) use a debugger. And just spoon-feeding you code is not going to teach you anything, so no, I'm not going to do that. – Jesper Juhl Nov 11 '18 at 15:47

2 Answers2

2

First point, you are using static arrays string strgs1[a-1], strgs2[b-1]; with sizes non-constant at compile time. That's a bad idea. I would advice to use std::vector instead.

Second point, the sizes that you are using for your arrays are not good. For example, the size of your first array is a-1 and you try to insert a strings inside it (from 0 to a-1).

sowlosc
  • 470
  • 3
  • 8
  • "That's a bad idea" - not just a bad idea. Such arrays are *not* valid C++ at all. Some compilers implement VLAs as an extension, but it's *not* valid according to the standard and *will* break on other compilers. – Jesper Juhl Nov 11 '18 at 15:22
  • I thought array size also starts from 0 that's why I wrote strgs1[a-1]. Now the code is working thanks! I don't know std::vector I will learn it. now the question seems very stupid..lol – Manik Nov 11 '18 at 16:45
0

Suppose you enter a=3. The size of the array is a-1=2. The loop iterates i=0, i=1, i=2. But this is 3 elements, while your vector has only size 2!

Furthermore, use std::vector. Arrays with non constant size are allowed by some compilers, but it is not portable.

Fabio
  • 2,105
  • 16
  • 26