1
\$\begingroup\$

I have been told that using namespace std is a bad practice (especially in header files). But wouldn't this make the program less readable?

int main()
{
    std::string text;
    std::vector<std::string> svec;

    while(std::cin >> text)
        svec.push_back(text);

    for(std::vector<std::string>::size_type i = 0; i < svec.size(); i++)
        std::cout << svec[i]  + " " << svec[i].size() << "\n";

    return 0;
}

Does this not make the for loop difficult to read, even for a simple program as this? Is there a better way to do this?

\$\endgroup\$
7
\$\begingroup\$

I'll assume that you've read Why is using namespace std considered bad practice? so you understand the reasons that this can cause problems for your code. I'm not going to address that any further in this answer.

You have alternatives, that reduce the risk compared to using namespace std; at file scope:

  • Selectively import names you're going to use, in the scope you're going to use them:

    #include <string>
    #include <vector>
    #include <iostream>
    
    int main()
    {
        using std::string;
        using std::vector;
    
        string text;
        vector<string> svec;
    
        while (std::cin >> text)
            svec.push_back(text);
    
        for (vector<string>::size_type i = 0; i < svec.size(); i++)
            std::cout << svec[i]  + " " << svec[i].size() << "\n";
    
        return 0;
    }
    

    This is an important technique when using free functions such as begin() and end() in generic code, where you want the std:: implementations to be available, but for argument-dependent lookup to prefer a more specific (local namespace) override if available.

  • Reduce the need to actually write typenames, with greater use of auto. For example, your output loop can be

    for (const auto& element: svec)
        std::cout << element << " " << element.size() << "\n";
    

    Using auto more liberally can be very helpful when you decide that a method needs to be generic; you won't need to trawl through it updating the types to match the new signature.

As an aside, note that some namespaces (e.g. std::literals and its contained namespaces) are intended to be imported with a using namespace directive; you may still want to restrict their scope to just that part of your code that needs it, though - and never in a header file!

\$\endgroup\$
  • \$\begingroup\$ I still consider that bad practice. You now have to use the using std::vector in every block or your code is inconsistent across functions. So you have not really improved things. A better technique is to either use auto for the iterator type (which it was designed for) or use the range based for(:) \$\endgroup\$ – Martin York Mar 8 '17 at 20:28
7
\$\begingroup\$

Namespaces

At least in my experience, dislike for the std:: prefix all over the place is largely a matter of what you're accustomed to seeing. When namespaces were new, I found it quite jarring and off-putting. 20 years (or so) later, I generally prefer code that includes it (though some might argue that this is just a strange case of Stockholm syndrome).

In this case, if you really prefer to avoid the std:: all over the place, you could put the namespace directive inside of main, to get something like this:

int main()
{
    using namespace std;

    string text;
    vector<string> svec;

    while (cin >> text)
        svec.push_back(text);

    for (vector<string>::size_type i = 0; i < svec.size(); i++)
        cout << svec[i] + " " << svec[i].size() << "\n";

    return 0;
}

If you prefer that, I doubt that anybody's likely to object too strongly to a namespace directive whose scope is limited to a single function.

Algorithms

Regardless of how you choose to deal with the namespace issue, I'd at least consider using a standard algorithm instead of writing explicit loops yourself.

In addition, your code isn't particularly efficient in its use of memory--its basically "parking" data in the vector for no particular reason. That is, it copies data from input to the vector, then from the vector to output, but never does anything with it in between that requires that the strings be stored.

That being the case, I'd avoid storing more data than necessary, and I'd use a standard algorithm to do the work.

#include <algorithm>
#include <string>
#include <iostream>
#include <iterator>

int main(int argc, char const *argv[])
{
    using namespace std;

    istream_iterator<string> in{ cin }, end;

    transform(in, end, ostream_iterator<string>(cout, "\n"),
        [](string const &s) { return s + ' ' + to_string(s.length()); });
}
\$\endgroup\$
  • 1
    \$\begingroup\$ I had just written a comment exactly to this effect and then deleted it when I saw your much more thorough examples. Thanks! \$\endgroup\$ – Edward Mar 8 '17 at 18:34
  • 1
    \$\begingroup\$ I object to it. Now you have to do it inside every function or your code is inconsistent. \$\endgroup\$ – Martin York Mar 8 '17 at 20:30
  • \$\begingroup\$ In general algorithm is the correct solution. For this particular case (its too simple) not so much. \$\endgroup\$ – Martin York Mar 8 '17 at 20:40
  • \$\begingroup\$ Wouldn't it be better to use std::for_each and bypass string construction? \$\endgroup\$ – Snowhawk Mar 8 '17 at 20:42
5
\$\begingroup\$

Using a range-based for loop makes the loop much easier to read

#include <iostream>
#include <vector>

int main()
{
    std::string text;
    std::vector<std::string> vector;

    while(std::cin >> text) vector.push_back(text);

    for (const auto& value : vector)
    {
        std::cout << value << ' ' << value.size() << '\n';
    }
}
\$\endgroup\$
  • \$\begingroup\$ I haven't come across const auto &value : vector , I want to know more about its working, where can I read about it ? \$\endgroup\$ – CaptainDaVinci Mar 8 '17 at 15:16
  • \$\begingroup\$ There is a description of auto iterators here en.cppreference.com/w/cpp/language/range-for. \$\endgroup\$ – pacmaninbw Mar 8 '17 at 15:18
  • 1
    \$\begingroup\$ @CaptainDaVinci, it is called range loop. It has roughly the following syntax for (T element: sequence) {...}. And the const auto& value is just a variable declaration, and auto tells compiler to deduce the type for you. Be aware though, type deduction is not always that obvious, sometimes it might do some surprising things. \$\endgroup\$ – Incomputable Mar 8 '17 at 16:45
3
\$\begingroup\$

SO your issue is that this is not as readable:

for(std::vector<std::string>::size_type i = 0; i < svec.size(); i++)
    std::cout << svec[i]  + " " << svec[i].size() << "\n";

Well I disagree with that. BUT there are also better ways to write that:

for(auto const& item: svec.size()) {
    std::cout << item  + " " << item.size() << "\n";
}

// Or 
for(auto loop = std::cbegin(svec); loop != std::cend(svec); ++svec) {
    std::cout << *loop  + " " << loop->size() << "\n";
}

// Or if you must use an index
for(auto loop = 0UL; loop < svec.size(); ++loop) {
    std::cout << svec[loop]  + " " << svec[loop].size() << "\n";
}

Or rather than even doing manual loops use an algorithm. They can make your code more expressive:

std::for_each(std::cbegin(svec), std::cend(svec),
              [](auto const& item){std::cout << item << " " << item.size() << "\n";});


// Notice the `PrintStringAndSize` expresses my intent.
// Admittedly for this situation it is "WAY overkill" so look
// at this as a suggestion for more complex code situations.
//
std::copy(std::cbegin(svec), std::cend(svec),
          std::ostream_iterator<PrintStringAndSize>(std::cout));



// Somewhere else
struct PrintStringAndSize
{
    std::string const& s;
    PrintStringAndSize(std::string const& s) : s(s) {}
    friend std::ostream& operator<<(std::ostream& str, PrintStringAndSize const& item) {
        return s << item.s << ' ' << item.s.size() << "\n";
    }
};
\$\endgroup\$
2
\$\begingroup\$

No love for typedefs?

typedef std::vector<std::string> MyVec;
typedef std::map<std::string, std::vector<std::set<std::string> > > CrazyContainer;

...
for(MyVec::size_type i = 0; i < svec.size(); i++)
    std::cout << svec[i]  + " " << svec[i].size() << "\n";

// Or for C++11

using MyVec          = std::vector<std::string;
using CrazyContainer = std::map<std::string, std::vector<std::set<std::string> > >;

...
for(MyVec::size_type i = 0; i < svec.size(); i++)
    std::cout << svec[i]  + " " << svec[i].size() << "\n";

Using auto is nice and should definitely be encouraged, and range-based for is nice, but I don't think quite answers the question being asked.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the additional info @LokiAstari. I've barely started with C++11 and haven't even scratched 14 yet. What is the advantage of the newer syntax? Is it just readability? \$\endgroup\$ – thegreatemu Mar 9 '17 at 18:24
  • \$\begingroup\$ It was introduced as a usability thing. We (in the west) are used to putting the thing we are defining on the left hand side. The typedef had the opposite format. It is just a way to make the language more consistent. There is an advantage in more complex situations where you can templatize the using where you can't templatize a typedef \$\endgroup\$ – Martin York Mar 9 '17 at 18:30
  • \$\begingroup\$ stackoverflow.com/q/10747810/14065 \$\endgroup\$ – Martin York Mar 9 '17 at 18:31
1
\$\begingroup\$

Adding the namespace identifies where (what library) the function is defined. This seems unimportant to you now because you're a beginner. As your software gets more complex it becomes very important.

This has been answered many time on stackoverflow.

\$\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.