Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extracted interfaces from UndoService and ChangeFactory for IoC container registration #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danjoconnell
Copy link

Hi Nathan,

I was wanting to use Monitored Undo Framework in a Prism based project I’m working on, but Prism’s IoC functionality (like most others) seems to require services to have an interface abstraction for container registration and dependency injection.

To facilitate this, in this this pull request I’ve extracted interfaces from UndoService and ChangeFactory and moved XML documentation comments to the interfaces to avoid repetition.
These interfaces also make it easier to mock UndoService and ChangeFactory in unit tests.

With these changes I can register UndoService and ChangeFactory with the IoC container by adding the following lines to the overriden RegisterTypes method in my application’s App.xaml.cs:

containerRegistry.RegisterInstance<MonitoredUndo.IUndoService>(MonitoredUndo.UndoService.Current);
containerRegistry.RegisterSingleton<MonitoredUndo.IChangeFactory, MonitoredUndo.ChangeFactory>();

Thank you for your time and consideration.

Regards,

Daniel O’Connell

… documentation comments to the interfaces to avoid repetition. The new IUndoService and IChangeFactory interfaces allow easier IoC container injection as well as the ability to mock in unit tests.
@nathanaw
Copy link
Owner

Thanks Daniel!
Will take a look this week.

@nathanaw
Copy link
Owner

A few questions and observations

  1. I assume you're using Unity as the container for Prism? If so, I think you can use that with concrete types. I did a small test (below).
  2. In reviewing the codebase, I think there are a few more changes we'd need if we moved it to an interface. There is code in the library that uses the UndoService.Current. That code would need to change so that it honored the interface. But that gets complicated, because we'd need to supply a mechanism to wire these up.
    • See UndoBatch line 32
    • See ChangeFactory lines 80 and 227
void Main()
{
	var undoService = MonitoredUndo.UndoService.Current;
	
	var container = new Unity.UnityContainer();
	
	container.RegisterInstance(undoService);
        // OR
	//container.RegisterFactory<UndoService>(c => undoService, FactoryLifetime.Singleton);
	
	var resolved = container.Resolve<UndoService>();
	Console.WriteLine("Resolved same container = {0}", ReferenceEquals(undoService, resolved));
}

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.

2 participants