Suggestions on AddWatchCommand impl

Mar 26, 2008 at 6:26 PM
Hi,

The implementation of AddWatchCommand seems non-standard and flawed to me. If multiple callers invoke SetFiredCallback then the last caller wins. Wouldn't it make more sense to just define an Executed event? I tried this out and here's the resultant code:

//TickerSymbolEventArgs.cs
public sealed class TickerSymbolEventArgs : EventArgs
{
	public string TickerSymbol { get; private set; }
 
	public TickerSymbolEventArgs(string tickerSymbol)
	{
		TickerSymbol = tickerSymbol;
	}
}
 
//AddWatchCommand.cs
public class AddWatchCommand : ICommand
{
	private EventHandler<TickerSymbolEventArgs> _executedHandler;
 
	public event EventHandler<TickerSymbolEventArgs> Executed
	{
		add
		{
			_executedHandler = Delegate.Combine(_executedHandler, value) as EventHandler<TickerSymbolEventArgs>;
			OnCanExecuteChanged();
		}
		remove
		{
			_executedHandler = Delegate.Remove(_executedHandler, value) as EventHandler<TickerSymbolEventArgs>;
			OnCanExecuteChanged();
		}
	}
 
	private void OnCanExecuteChanged()
	{
		if (CanExecuteChanged != null)
		{
			CanExecuteChanged(this, EventArgs.Empty);
		}
	}
 
    public bool CanExecute(object parameter)
    {
        return _executedHandler != null;
    }
 
    public event EventHandler CanExecuteChanged;
 
    public void Execute(object parameter)
    {
        if (_executedHandler != null)
        {
			_executedHandler(this, new TickerSymbolEventArgs(parameter as string));
        }
    }
}
 
//WatchListService.cs
WatchListModuleCommands.AddWatchCommand.Executed += AddWatchCommand_Executed;
 
...
 
private void AddWatchCommand_Executed(object sender, TickerSymbolEventArgs e)
{
	if (!String.IsNullOrEmpty(e.TickerSymbol))
	{
		string upperCasedTrimmedSymbol = e.TickerSymbol.ToUpperInvariant().Trim();
 
		if (!WatchItems.Contains(upperCasedTrimmedSymbol))
		{
			if (marketFeedService.SymbolExists(upperCasedTrimmedSymbol))
			{
				WatchItems.Add(upperCasedTrimmedSymbol);
			}
		}
	}
}

Think I'll have more comments when I dig deeper, but this one just jumped out at me.

Cheers,
Kent