Added namespace and include#988
Conversation
|
@gregdegruy : Thanks for your contribution! The author, @, has been notified to review your proposed change. |
|
@Saisang, can you approve this? |
mikeblome
left a comment
There was a problem hiding this comment.
@gregdegruy good thanks! @jborsecnik pls let corob-msft or myself review public PRs in cpp-docs except for simple typos which require no approval.
|
#hold-off |
|
@gregdegruy 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 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-upThe 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 This change is a bit more ambitious, but arguably more useful. Do you want to make it? |
|
@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.
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.
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
|
|
The changes discussed were committed and merged independently, so I'm closing this reminder. |
No description provided.