Is this code right? My IT teacher says it's wrong

Question

Take a string from user then change the last letter to upper and write the 2nd letter and 4th letter from the string.

Generate 10 random integers from <-10,5>. Print these numbers. To the even numbers, add 100 and print them again.

I wrote this code; is it right?

#include <iostream>
#include <string>
#include<time.h>
#include <array>
using namespace std;
int main()
{
    string a;
    cin >> a;
    cout << a << endl;
    a.back() = toupper(a.back());
    cout << a<<endl;
    cout << "2 letter is "<<a.at(2) << endl;
    cout <<" 4 letter is "<< a.at(4) << endl;


    srand(unsigned(time(0)));
    int tab[10];
    for (int i = 0; i < 10; i++)
    {
        int z = (rand() % 16) - 10;
        tab[i] = z;
    }
    for (int i = 0; i < 10; i++)
    {
        cout << tab[i] << endl;
    }
    cout << "===============" << endl;
    for (int i = 0; i < 10; i++)
    {
        if (tab[i] % 2 == 0)
        {
            tab[i] += 100;
        }
    }
    for (int i = 0; i < 10; i++)
    {
        cout << tab[i] << endl;
    }
}

There is no such thing as the right code or the wrong code. All code is equal and the only thing that matters is how you FEEL about your code! So unless you clarify what you want, no one will be able to help you. — , Apr 30 '20 at 13:37
Could be an assignment with automated tests, that tells you pass/fail before the deadline and afterwards is more helpful. That avoids `std::cout << static_correct_answers;` — Caleth, Apr 30 '20 at 13:44

score 3 · Answer 1 · answered Apr 30 '20 at 13:39

3

This is wrong

cout << "2 letter is "<< a.at(2) << endl;

The second letter in a string is a.at(1) because in C++ array indexes start at zero, but in English we start counting at one.

answered Apr 30 '20 at 13:39

john

85,011
4
57
81

score 3 · Answer 2 · answered Apr 30 '20 at 13:40

3

Your code is wrong! The following two lines, in particular:

    cout << "2 letter is "<<a.at(2) << endl;
    cout <<" 4 letter is "<< a.at(4) << endl;

Why? Because array indexes (and string positions) begin at zero in C++, so the first letter will be a.at(0) and, thus, the second letter will be a.at(1).

So, you should have this:

    cout << "2 letter is " << a.at(1) << endl;
    cout <<" 4 letter is " << a.at(3) << endl;

Also, for better overall coding-style guidelines, please read this: Why is "using namespace std;" considered bad practice?

answered Apr 30 '20 at 13:40

Adrian Mole

49,934
160
51
83

2

I emphatically disagree with "don't use namespace std" for small homework programs like this one. Cluttering the program with wordy and redundant `std::` prefixes does nothing but create noise which distracts from the actual thing the program does. The reason given by the accepted and most upvoted answer there are namespace conflicts when "using" two namespaces, which simply is not the case here (**and** is detected by the compiler **and** is trivially fixed by prefixing with `std::`...) , so the answer does not apply here at all. (Things are different in large projects, of course.) – Peter - Reinstate Monica Apr 30 '20 at 13:47
@Peter I understand your point-of-view, yet I *still* maintain that it is bad practice to use that 'generic' namespace statement - even for "small homework problems." It is in such exercises that habits are formed ... and these should be *good* habits. Adding lines like `using std::string` and `using std::cout` (etc.) is far better, IMHO. – Adrian Mole Apr 30 '20 at 13:51
To a degree, I concur. But there is also the meta thing to learn "use the right tools and methods for the job." ;-) Oh, and I think `using` for a class is relatively new, isn't it? – Peter - Reinstate Monica Apr 30 '20 at 13:53
@Peter-ReinstateMonica I'm not trying to be spiteful, or anything like that, but my mind came back to your comments when I just answered [this question](https://stackoverflow.com/q/61564027/10871073). ‎ – Adrian Mole May 02 '20 at 18:13
That's OK. With respect to the post you link: The OP there does not define a swap for std::strings, or it would have been chosen over a template (iirc the rules correctly), "using" namespace std or not... – Peter - Reinstate Monica May 02 '20 at 19:02

score 1 · Answer 3 · answered Apr 30 '20 at 13:39

    for (int i = 0; i < 10; i++)
    {
        if (tab[i] % 2 == 0)
        {
            tab[i] += 100;
        }
    }
    for (int i = 0; i < 10; i++)
    {
        cout << tab[i] << endl;
    }

I believe this is where your problem lies. Your teacher said if they are even, print them again - and by that I think he/she means to print just the even numbers again (I am taking a liberal use of language here), so change the above to:

    for (int i = 0; i < 10; i++)
    {
        if (tab[i] % 2 == 0)
        {
            tab[i] += 100;
            cout << tab[i] << endl;
        }
    }

She mean all with this +=100 – WujcioKarol Apr 30 '20 at 13:41 — WujcioKarol, Apr 30 '20 at 13:41

score 0 · Answer 4 · answered Apr 30 '20 at 13:54

There are several things I would consider sub-optimal or incorrect if I was your instructor:

Accessing at(2) and at(4) will give back 3rd and 5th characters, not 2nd and 4th. Also, when you say "2 letter" and "4 letter", do you (or your instructor) mean letter #2 and letter #4, or first 2 letters and then first 4 letters. There seems to be something lost in translation here, as was already commented.
You do not check for string length before accessing characters in it. You access a.back() without checking that the string is not empty, and at(2) and at(4) without checking it is of sufficient length. This makes it very easy to crash your program, by providing insufficient characters as input (you did not provide any input assumptions, so this may be OK with your instructor).
For the second part of the program - the one with the random numbers - you are using 4 loops, when 2 would have been perfectly sufficient: you could generate the numbers, and print them in one loop. Then, you could add 100 to the even ones, and print again, in a second loop.

Finally, when writing code, be consistent with your indentation. The first part of you code is indented by 4 characters, and the second, by 8. Only indent within blocks, and keep indentation uniform for the entire block.

I fixed the indentation in the answer, in case anybody wonders. — Peter - Reinstate Monica, Apr 30 '20 at 14:13

Is this code right? My IT teacher says it's wrong

4 Answers4