Update com-callable-wrapper.md #8993
Conversation
line 81-93: fixed code syntax line 138-148: fixed code syntax line 167-177: fixed code syntax
|
@AaronRobinsonMSFT and I are going to review this PR and document together in the new week. I read most of the doc and found multiple things that could be improved. Please ping me if you want to be part of this. This is also a good time to decide what we're going to do w/rt COM Interop docs for the upcoming .NET Core 3.0 release, which will support this feature/scenario. |
|
Hello, @richlander and @AaronRobinsonMSFT ! |
| // Implicitly extends System.Object. | ||
| public class Mammal | ||
| { | ||
| public Mammal() {} |
There was a problem hiding this comment.
If a class doesn't define any other constructor, it will have a default public parameterless constructor. So I don't think adding the constructor here and also below is necessary.
There was a problem hiding this comment.
You are right, this default public parameterless constructor has been added only as a visual example for perception. It can be deleted.
| { | ||
| public LoanApp() {} | ||
|
|
||
| Int32 IExplicit.M() { return 0; } |
There was a problem hiding this comment.
I think it's more idiomatic to use int, rather than Int32. Also, you could use expression-bodied method here:
| Int32 IExplicit.M() { return 0; } | |
| int IExplicit.M() => 0; |
The same applies for the block below.
There was a problem hiding this comment.
Here I would choose a classic implementation. Because the 'return' instruction is more recognizable and identifiable.
There was a problem hiding this comment.
@sharpist I think the 'classic implementation' argument is valid here, but the type really should be int.
There was a problem hiding this comment.
@AaronRobinsonMSFT this can be changed.
Because these two ways of specifying the return type are equivalent.
I replaced return type 'Int32' -> 'int' and 'default public parameterless constructors' now deleted
|
|
||
| ```csharp | ||
| // The CLR does not expose a class interface for this type. | ||
| // COM clients can call the members of this class using the methods from the IExplicit interface. |
There was a problem hiding this comment.
This comment is in the C# snippet, so readers with Visual Basic as the selected language will miss that comment. Moreover, comments are not translated, so this content won't be localized. @sharpist please consider moving this paragraph into the body of the article.
The same is applicable to another snippet update.
There was a problem hiding this comment.
@pkulikov I think these both accompanying comments can be excluded from the code examples. At your discretion. Because the main article already contains a detailed description.
There was a problem hiding this comment.
I think these both accompanying comments can be excluded from the code examples.
If the article contains enough explanations, I also don't see the need in the code comments.
There was a problem hiding this comment.
@pkulikov Are you happy with the latest change? I am. I think it's clearer with the explicit interface.
Removed unnecessary comments from edited code examples.
|
@AaronRobinsonMSFT and I were going to meet at 10AM PST on Monday (tomorrow). That looks like the best time (10AM) we can select. DM me on twitter @runfaster2000 and we can work out particulars. |
|
Hello, @richlander and Everyone here! I'm only from time to time on twitter but thanks for your twitter and I've seen some your interesting notes on the topic. |
6 participants
line 81-93: fixed code syntax
line 138-148: fixed code syntax
line 167-177: fixed code syntax