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

style: tidy up comments for ReactiveBinding and command binding #1267

Merged
merged 2 commits into from Feb 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 37 additions & 2 deletions src/ReactiveUI/CommandBinding.cs
Expand Up @@ -9,6 +9,9 @@

namespace ReactiveUI
{
/// <summary>
/// Various helpers to bind View controls and ViewModel commands together
/// </summary>
public static class CommandBinder
{
static ICommandBinderImplementation binderImplementation;
Expand All @@ -30,7 +33,7 @@ static CommandBinder()
/// <param name="view">The View</param>
/// <param name="viewModel">The View model</param>
/// <param name="controlName">The name of the control on the view</param>
/// <param name="propertyName">The ViewModel command to Bind.</param>
/// <param name="propertyName">The ViewModel command to bind.</param>
/// <param name="withParameter">The ViewModel property to pass as the
/// param of the ICommand</param>
/// <param name="toEvent">If specified, bind to the specific event
Expand All @@ -57,6 +60,7 @@ static CommandBinder()
/// the binding</returns>
/// <param name="view">The View</param>
/// <param name="viewModel">The View model</param>
/// <param name="propertyName">The ViewModel command to bind</param>
/// <param name="controlName">The name of the control on the view</param>
/// <param name="toEvent">If specified, bind to the specific event
/// instead of the default.</param>
Expand All @@ -81,6 +85,7 @@ static CommandBinder()
/// the binding</returns>
/// <param name="view">The View</param>
/// <param name="viewModel">The View model</param>
/// <param name="propertyName">The ViewModel command to bind</param>
/// <param name="controlName">The name of the control on the view</param>
/// <param name="withParameter">The ViewModel property to pass as the
/// param of the ICommand</param>
Expand Down Expand Up @@ -126,8 +131,25 @@ interface ICommandBinderImplementation : IEnableLogger
where TProp : ICommand;
}

/// <summary>
/// Used by the CommandBinder extension methods to handle binding View controls and ViewModel commands
/// </summary>
public class CommandBinderImplementation : ICommandBinderImplementation
{
/// <summary>
/// Bind a command from the ViewModel to an explicitly specified control
/// on the View.
/// </summary>
/// <returns>A class representing the binding. Dispose it to disconnect
/// the binding</returns>
/// <param name="view">The View</param>
/// <param name="viewModel">The View model</param>
/// <param name="controlProperty">The name of the control on the view</param>
/// <param name="vmProperty">The ViewModel command to bind</param>
/// <param name="withParameter">The ViewModel property to pass as the
/// param of the ICommand</param>
/// <param name="toEvent">If specified, bind to the specific event
/// instead of the default.</param>
public IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
TViewModel viewModel,
TView view,
Expand Down Expand Up @@ -157,6 +179,20 @@ public class CommandBinderImplementation : ICommandBinderImplementation
source, BindingDirection.OneWay, bindingDisposable);
}

/// <summary>
/// Bind a command from the ViewModel to an explicitly specified control
/// on the View.
/// </summary>
/// <returns>A class representing the binding. Dispose it to disconnect
/// the binding</returns>
/// <param name="view">The View</param>
/// <param name="viewModel">The View model</param>
/// <param name="controlProperty">The name of the control on the view</param>
/// <param name="vmProperty">The ViewModel command to bind</param>
/// <param name="withParameter">The ViewModel property to pass as the
/// param of the ICommand</param>
/// <param name="toEvent">If specified, bind to the specific event
/// instead of the default.</param>
public IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
TViewModel viewModel,
TView view,
Expand Down Expand Up @@ -303,7 +339,6 @@ public static IDisposable BindCommandToObject(ICommand command, object target, I
var mi = binder.GetType().GetTypeInfo().DeclaredMethods.First(x => x.Name == "BindCommandToObject" && x.IsGenericMethod);
mi = mi.MakeGenericMethod(new[] {eventArgsType});

//var ret = binder.BindCommandToObject<TEventArgs>(command, target, commandParameter, eventName);
var ret = (IDisposable) mi.Invoke(binder, new[] {command, target, commandParameter, eventName});
if (ret == null) {
throw new Exception(String.Format("Couldn't bind Command Binder for {0} and event {1}", type.FullName, eventName));
Expand Down
16 changes: 2 additions & 14 deletions src/ReactiveUI/ReactiveBinding.cs
Expand Up @@ -18,7 +18,7 @@ public interface IReactiveBinding<TView, TViewModel, TValue> : IDisposable
where TView : IViewFor
{
/// <summary>
/// The instance of the view model this binding is applied to.</param>
/// The instance of the view model this binding is applied to.
Copy link
Contributor

Choose a reason for hiding this comment

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

lolwat - good catch!

/// </summary>
/// <value>
/// The view model.
Expand Down Expand Up @@ -76,15 +76,6 @@ internal class ReactiveBinding<TView, TViewModel, TValue> : IReactiveBinding<TVi
{
private IDisposable bindingDisposable;

/// <summary>
/// Initializes a new instance of the <see cref="AppliedBindingInfo{TViewModel}" /> class.
/// </summary>
/// <param name="view">The view.</param>
/// <param name="viewModel">The view model.</param>
/// <param name="viewPath">The view path.</param>
/// <param name="viewModelPath">The view model path.</param>
/// <param name="direction">The direction.</param>
/// <param name="bindingDisposable">The binding disposable.</param>
public ReactiveBinding(TView view, TViewModel viewModel, Expression viewExpression, Expression viewModelExpression,
IObservable<TValue> changed, BindingDirection direction, IDisposable bindingDisposable)
{
Expand All @@ -99,7 +90,7 @@ internal class ReactiveBinding<TView, TViewModel, TValue> : IReactiveBinding<TVi
}

/// <summary>
/// The instance of the view model this binding is applied to.</param>
/// The instance of the view model this binding is applied to.
/// </summary>
/// <value>
/// The view model.
Expand Down Expand Up @@ -150,9 +141,6 @@ internal class ReactiveBinding<TView, TViewModel, TValue> : IReactiveBinding<TVi
/// </value>
public BindingDirection Direction { get; private set; }

/// <summary>
/// Releases unmanaged and - optionally - managed resources.
/// </summary>
public void Dispose()
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, where did this originate from? Looks like some incredibly useful ghostdoc vomit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really know...it seemed a bit like stating the obvious. To be honest, I'm not sure any of these comments make sense on the internal implementation of the fairly well commented interface...

Copy link
Member

Choose a reason for hiding this comment

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

GhostDoc noise can burn in a pit of 🔥

{
if (bindingDisposable != null) {
Expand Down