A VS MPF bug, about how to write good virtual fucntion
2010-05-14 01:24
525 查看
MPF bug: creating the custom ProjectConfig class
Old design:
MPF ConfigProvider.cs:
namespace Microsoft.VisualStudio.Package
{
public class ConfigProvider : IVsCfgProvider2, IVsProjectCfgProvider, IVsExtensibleObject
{
private Dictionary<string, ProjectConfig> configurationsList = new Dictionary<string, ProjectConfig>();
85: protected virtual ProjectConfig CreateProjectConfiguration(string configName)
86: {
87: // if we already created it, return the cached one
88: if (configurationsList.ContainsKey(configName))
89: return configurationsList[configName];
90:
91: ProjectConfig requestedConfiguration = new ProjectConfig(this.project, configName);
92: configurationsList.Add(configName, requestedConfiguration);
93:
94: return requestedConfiguration;
95: }
Now let’s imagine that developer wants to inherit ProjectConfig class with custom one (for example to customize its DebugLaunch logic) and he/she overrides CreateProjectConfiguration method to return custom MyProjectConfig. At the same time he/she wants to stay configuration caching logic.
But… It is imposible due to configurationsList has ‘private’ accessor in base class! :
protected override ProjectConfig CreateProjectConfiguration(string configName)
{
// if we already created it, return the cached one
if (configurationsList.ContainsKey(configName))
{
return configurationsList[configName];
}
AsProjectConfig requestedConfiguration = new MyProjectConfig(ProjectMgr, configName);
configurationsList.Add(configName, requestedConfiguration);
return requestedConfiguration;
}
RESOLUTION:
Way 1: Change ‘private’ to at least ‘protected’ for :
private Dictionary<string, ProjectConfig> configurationsList = new Dictionary<string, ProjectConfig>();
It’s simple but it ‘smells bad’.
Way 2: Rename current ProjectConfig.CreateProjectConfiguration method to GetProjectConfiguration and extract the real CreateProjectConfiguration method which is really creates the ProjectConfig instance and doen’t perform any other operations.
85: protected virtual ProjectConfig GetProjectConfiguration (string configName)
86: {
87: // if we already created it, return the cached one
88: if (configurationsList.ContainsKey(configName))
89: return configurationsList[configName];
90:
91: ProjectConfig requestedConfiguration = CreateProjectConfiguration(this.project, configName);
92: configurationsList.Add(configName, requestedConfiguration);
93:
94: return requestedConfiguration;
95: }
96:
97: protected virtual ProjectConfig CreateProjectConfiguration(ProjectNode project, string configName)
98: {
99: return new ProjectConfig(this.project, configName);
100: }
That will be a good resolution!
P.S. also posted here - http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=1371036&SiteID=1&mode=1
Thanks
======Mail discussion:
I agree that the comments would have been better saying something like “get a configuration…” instead of “Create….
The pattern really is that you should only call the getter and not the Create.. method. The Create method is your abstract factory method that you should override to handle your own special Project Configuration types.
I will fix that the next time I make some changes to the project sources and thanks for the feedback.
Ole
From:
Sent: Thursday, April 10, 2008 12:42 AM
I think the fix is ok except for the comment.
The comment I prefer is something like: Get an existing Project Configuration according to the configName if configName already exists in configuration list, otherwise it returns a new one.
From:
Sent: 2008年4月10日 14:13
To: Ole Preisler (VS SDK)
Hi Ole
Sorry to trouble you.
This bug: MPF bug: creating the custom ProjectConfig class is fixed by you. I have closed it.
And I have a little doubts with the bug fix.
Now the file ConfigProvider.cs has been changed to:
So if we override CreateprojectConfiguration, and creating a custom ProjectConfig class MyProjectConfig.
protected override ProjectConfig CreateProjectConfiguration(string configName)
{
MyProjectConfig requestedConfiguration = new MyProjectConfig(ProjectMgr, configName);
return requestedConfiguration;
}
If want to create and add it to configurationsList. We have to use GetProjectConfiguration
1.
ConfigProvider myConfigProvider = new ConfigProvider(myProjectNode);
myConfigProvider.CreateProjectConfiguration(“myConfig”); // can’t add it to configurationsList
2.
ConfigProvider myConfigProvider = new ConfigProvider(myProjectNode);
myConfigProvider.GetProjectConfiguration(“myConfig”); // Ok to new and add it to configurationsList
I think method 2 has some problems in logic. We use Get* method to new Configuration. But it seems that we have no better workaround.
Old design:
MPF ConfigProvider.cs:
namespace Microsoft.VisualStudio.Package
{
public class ConfigProvider : IVsCfgProvider2, IVsProjectCfgProvider, IVsExtensibleObject
{
private Dictionary<string, ProjectConfig> configurationsList = new Dictionary<string, ProjectConfig>();
85: protected virtual ProjectConfig CreateProjectConfiguration(string configName)
86: {
87: // if we already created it, return the cached one
88: if (configurationsList.ContainsKey(configName))
89: return configurationsList[configName];
90:
91: ProjectConfig requestedConfiguration = new ProjectConfig(this.project, configName);
92: configurationsList.Add(configName, requestedConfiguration);
93:
94: return requestedConfiguration;
95: }
Now let’s imagine that developer wants to inherit ProjectConfig class with custom one (for example to customize its DebugLaunch logic) and he/she overrides CreateProjectConfiguration method to return custom MyProjectConfig. At the same time he/she wants to stay configuration caching logic.
But… It is imposible due to configurationsList has ‘private’ accessor in base class! :
protected override ProjectConfig CreateProjectConfiguration(string configName)
{
// if we already created it, return the cached one
if (configurationsList.ContainsKey(configName))
{
return configurationsList[configName];
}
AsProjectConfig requestedConfiguration = new MyProjectConfig(ProjectMgr, configName);
configurationsList.Add(configName, requestedConfiguration);
return requestedConfiguration;
}
RESOLUTION:
Way 1: Change ‘private’ to at least ‘protected’ for :
private Dictionary<string, ProjectConfig> configurationsList = new Dictionary<string, ProjectConfig>();
It’s simple but it ‘smells bad’.
Way 2: Rename current ProjectConfig.CreateProjectConfiguration method to GetProjectConfiguration and extract the real CreateProjectConfiguration method which is really creates the ProjectConfig instance and doen’t perform any other operations.
85: protected virtual ProjectConfig GetProjectConfiguration (string configName)
86: {
87: // if we already created it, return the cached one
88: if (configurationsList.ContainsKey(configName))
89: return configurationsList[configName];
90:
91: ProjectConfig requestedConfiguration = CreateProjectConfiguration(this.project, configName);
92: configurationsList.Add(configName, requestedConfiguration);
93:
94: return requestedConfiguration;
95: }
96:
97: protected virtual ProjectConfig CreateProjectConfiguration(ProjectNode project, string configName)
98: {
99: return new ProjectConfig(this.project, configName);
100: }
That will be a good resolution!
P.S. also posted here - http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=1371036&SiteID=1&mode=1
Thanks
======Mail discussion:
I agree that the comments would have been better saying something like “get a configuration…” instead of “Create….
The pattern really is that you should only call the getter and not the Create.. method. The Create method is your abstract factory method that you should override to handle your own special Project Configuration types.
I will fix that the next time I make some changes to the project sources and thanks for the feedback.
Ole
From:
Sent: Thursday, April 10, 2008 12:42 AM
I think the fix is ok except for the comment.
The comment I prefer is something like: Get an existing Project Configuration according to the configName if configName already exists in configuration list, otherwise it returns a new one.
From:
Sent: 2008年4月10日 14:13
To: Ole Preisler (VS SDK)
Hi Ole
Sorry to trouble you.
This bug: MPF bug: creating the custom ProjectConfig class is fixed by you. I have closed it.
And I have a little doubts with the bug fix.
Now the file ConfigProvider.cs has been changed to:
/// Creates new Project Configuartion objects based on the configuration name. /// </summary> /// <param name="configName">The name of the configuration</param> /// <returns>An instance of a ProjectConfig object.</returns> protected ProjectConfig GetProjectConfiguration(string configName) { // if we already created it, return the cached one if (configurationsList.ContainsKey(configName)) { return configurationsList[configName]; } ProjectConfig requestedConfiguration = CreateProjectConfiguration(configName); configurationsList.Add(configName, requestedConfiguration); return requestedConfiguration; } protected virtual ProjectConfig CreateProjectConfiguration(string configName) { return new ProjectConfig(this.project, configName); } |
protected override ProjectConfig CreateProjectConfiguration(string configName)
{
MyProjectConfig requestedConfiguration = new MyProjectConfig(ProjectMgr, configName);
return requestedConfiguration;
}
If want to create and add it to configurationsList. We have to use GetProjectConfiguration
1.
ConfigProvider myConfigProvider = new ConfigProvider(myProjectNode);
myConfigProvider.CreateProjectConfiguration(“myConfig”); // can’t add it to configurationsList
2.
ConfigProvider myConfigProvider = new ConfigProvider(myProjectNode);
myConfigProvider.GetProjectConfiguration(“myConfig”); // Ok to new and add it to configurationsList
I think method 2 has some problems in logic. We use Get* method to new Configuration. But it seems that we have no better workaround.
相关文章推荐
- Talk about how to write good C# code from a bug
- A good blog about how to write an Hadoop MapReduce program in Python
- How to write a good bug report? Tips and Tricksa
- How to write a good bug report
- How to write a good bug report? Tips and Tricks
- How to write a good bug?
- How to write a good build system ?
- How to write a good design document for peer engineers (如何写一份给工程师看的技术文档)
- How to Write a Useful Bug Report
- Some examples about how to write anonymous method and lambda expression
- How to write a good design document for peer engineers (如何写一份给工程师看的技术文档)
- how to write a good api
- Learn how to write a Regular Expression (a very good article)
- How to Write a Useful Bug Report
- About:How to write the declaration of pointers and references rightly.
- How to write a good design document for peer engineers (如何写一份给工程师看的技术文档)
- 写出优秀论文How To Write A Great Essay About Anything
- How to Write a Useful Bug Report
- How to Write Good Unit Tests?
- How to write a good design document for peer engineers (如何写一份给工程师看的技术文档)