Skip to content

Added namespace and include#988

Closed
ugreg wants to merge 1 commit into
MicrosoftDocs:masterfrom
ugreg:patch-1
Closed

Added namespace and include#988
ugreg wants to merge 1 commit into
MicrosoftDocs:masterfrom
ugreg:patch-1

Conversation

@ugreg
Copy link
Copy Markdown

@ugreg ugreg commented May 5, 2019

No description provided.

@PRMerger8
Copy link
Copy Markdown
Contributor

@gregdegruy : Thanks for your contribution! The author, @, has been notified to review your proposed change.

@jborsecnik
Copy link
Copy Markdown
Contributor

@Saisang, can you approve this?

Copy link
Copy Markdown
Contributor

@mikeblome mikeblome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregdegruy good thanks! @jborsecnik pls let corob-msft or myself review public PRs in cpp-docs except for simple typos which require no approval.

@tfosmark tfosmark assigned mikeblome and unassigned Saisang May 6, 2019
@colin-home
Copy link
Copy Markdown
Contributor

#hold-off

@colin-home
Copy link
Copy Markdown
Contributor

@gregdegruy
First off, thanks for working on making our documentation better. To help us understand what you're proposing, I recommend you include comments in your PR submissions that explain and justify your submission: what you think is wrong, and how your submission is the best fix. Absent that, we just have to guess.

On consideration, I don't think this is the right thing to do here, or the similar changes you've made in the other PRs in this topic.

The problem as I see it is, the existing snippets don't follow our own guidelines that most code examples should be self-contained, runnable code. These code snippets are just a few lines that make readers guess at our intended declarations and usage.

On review, I note that adding the include directives in each snippet makes some of our intentions explicit, but it doesn't help turn these into runnable examples. The include directives would be out of place if the snippets were cut and pasted into some other code.

The other change is one that comes up regularly: whether to namespace-qualify names from the standard library everywhere, or to include the whole namespace with a using namespace std; statement. For reasons of white space economy, the docs usually choose the latter. There are arguments within the C++ developer community as to which is better from a software engineering standpoint. I personally lean toward qualifying everything, as full qualification is always recommended in header files. However, some heavy hitters in software engineering are fine with less typing in implementation files when it's clear to the compiler which name comes from where. With IntelliSense, you can hover over any name and see its fully-qualified declaration. The compiler will let you know if it finds a name ambiguous. These reasons are why I don't consider adding the namespace qualifications alone sufficient justification for changing the docs examples.

I think instead we need a common baseline for this topic, a bit of prologue that comes before the examples; something like this, just before the Example 1 heading:

Example set-up

The examples assume that you've included the required headers and declared the required types, as shown in this code example:

// shared_ptr-examples.cpp
// The following examples assume these declarations:
#include <algorithm>
#include <iostream>
#include <memory>
#include <string>
#include <vector>

struct MediaAsset 
{
    virtual ~MediaAsset() = default; // make it polymorphic
};

struct Song : public MediaAsset
{
    std::wstring artist;
    std::wstring title;
    Song(const std::wstring& artist_, const std::wstring& title_) :
        artist{ artist_ }, title{ title_ } {}
};

struct Photo : public MediaAsset
{
    std::wstring date;
    std::wstring location;
    std::wstring subject;
    Photo(
        const std::wstring& date_,
        const std::wstring& location_,
        const std::wstring& subject_) :
        date{ date_ }, location{ location_ }, subject{ subject_ } {}
};

using namespace std;

int main()
{
    // The examples go here, in order:
    // Example 1
    // Example 2
    // Example 3
    // Example 4
    // Example 5
    // Example 6
}

There'd need to be a couple of typos fixed in one example code snippet, but they'd otherwise compile as-is. In Example 4, there's a stray ) in line 3 , and my declaration of Photo would mean location_ should be location instead, for consistency.

This change is a bit more ambitious, but arguably more useful. Do you want to make it?

@ugreg
Copy link
Copy Markdown
Author

ugreg commented May 10, 2019

@corob-msft

Thanks for taking the time to consider my PR and speak through the intentions of it. Completely agree with your point here and because of this I agree includes should not used in the snippets.

On review, I note that adding the include directives in each snippet makes some of our intentions explicit, but it doesn't help turn these into runnable examples. The include directives would be out of place if the snippets were cut and pasted into some other code.

I love the idea of a prologue or even an epilogue that ties all the snippets together. I think it would be a great addition to the docs. I've seen this approach in some other Microsoft docs I reference. I think the value of a prologue is seeing all the code come together. Often times I find it difficult to see this and so do Microsoft partners I work with.

using namespace std; is up for much debate and I still personally don't have a good answer for when to and when not to use this in documentation. Even variable member naming like date_ versus m_date is up for debate and it all comes down to the style of the development team.

I thinking speaking to namespace qualification as a best practice to follow in production and development is good enough. I would like to see more awareness brought to this as using namespace std; is sometimes thought to new developers or students without explaining it's in production software to the global namespace.

There are arguments within the C++ developer community as to which is better from a software engineering standpoint. I personally lean toward qualifying everything, as full qualification is always recommended in header files

@colin-home
Copy link
Copy Markdown
Contributor

The changes discussed were committed and merged independently, so I'm closing this reminder.

@colin-home colin-home closed this May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants