Return to Answer

Like t3chb0t, I am not familiar with VBA, but I will mention a few things that caught my eye:

1. Lack of documentation. To me it is surprising, that you went through the trouble of documenting a class that is an event argument, but completely left out IAutoComplete interface, which plays a pivotal role in this entire thing. I can make an educated guess on what most of the members do but by no means it is obvious.

2. config.UserSettings.AutoCompleteSettings.AutoCompletes.FirstOrDefault - here you can use dictionary instead of doing a linear lookup.

3. autoComplete.IsEnabled = setting.IsEnabled; - you might want to consider replacing it with autoComplete.ApplySettings(setting). Even though it "leaks" settings class to the interface, it makes a job of extending and/or customizing settings much easier.

4. Personally, I prefer to split bool Execute into void Execute and bool CanExecute (what Microsoft does in ICommand interface). I think it separates concerns better and allows to override those methods individually. It also makes it clear, that bool is not the result of operation (success/failure) but an indication of whether or not operation can be executed in the first place (can execute/cannot execute).

5. What pissed me off about Resharper is how hard it was to just input a single {. Without the }. And how it then removes both braces when you try to remove the second one... AFAIR they did something about it in later versions, but I am not sure what (maybe I just got used to it). You seem to mimic this behavior but I would love to see it improved in some way. :) Maybe add a setting, that can toggle this feature (the delete part) on or off.

6. Another thought about DeleteAroundCaret: maybe DELETE should be handled internally by autocomplete classes? I mean if autocomplete is responsible for adding "output", it should also know how to remove it. At least it makes sense to me.

7. VBENativeServices.KeyDown += HandleKeyDown; - I think you need another abstraction layer on top of this event, that would track which keys are used where. From my experience handling key presses in hotkey-heavy apps quickly becomes a nightmare, if you use a single event for that. You should come up with a way to manage hotkeys and detect conflicts. When multiple services want to handle, say, DELETE key in different way, there should be a clear policy in place, that should decide which service should handle this key in the current context, or throw an exception if the hotkey is ambiguous.

Like t3chb0t, I am not familiar with VBA, but I will mention a few things that caught my eye:

1. Lack of documentation. To me it is surprising, that you went through the trouble of documenting a class that is an event argument, but completely left out IAutoComplete interface, which plays a pivotal role in this entire thing. I can make an educated guess on what most of the members do but by no means it is obvious.

2. config.UserSettings.AutoCompleteSettings.AutoCompletes.FirstOrDefault - here you can use dictionary instead of doing a linear lookup.

3. autoComplete.IsEnabled = setting.IsEnabled; - you might want to consider replacing it with autoComplete.ApplySettings(setting). Even though it "leaks" settings class to the interface, it makes a job of extending and/or customizing settings much easier.

4. Personally, I prefer to split bool Execute into void Execute and bool CanExecute (what Microsoft does in ICommand interface). I think it separates concerns better and allows to override those methods individually. It also makes it clear, that bool is not the result of operation (success/failure) but an indication of whether or not operation can be executed in the first place (can execute/cannot execute).

5. What pissed me off about Resharper is how hard it was to just input a single {. Without the }. And how it then removed both braces when you tried to remove the second one... AFAIR they did something about it in later versions, but I am not sure what (maybe I just got used to it). You seem to mimic this behavior but I would love to see it improved in some way. :) Maybe add a setting, that can toggle this feature (the delete part) on or off.

6. Another thought about DeleteAroundCaret: maybe DELETE should be handled internally by autocomplete classes? I mean if autocomplete is responsible for adding "output", it should also know how to remove it. At least it makes sense to me.

7. VBENativeServices.KeyDown += HandleKeyDown; - I think you need another abstraction layer on top of this event, that would track which keys are used where. From my experience handling key presses in hotkey-heavy apps quickly becomes a nightmare, if you use a single event for that. You should come up with a way to manage hotkeys and detect conflicts. When multiple services want to handle, say, DELETE key in different way, there should be a clear policy in place, that should decide which service should handle this key in the current context, or throw an exception if the hotkey is ambiguous.

Like t3chb0t, I am not familiar with VBA, but I will mention a few things that caught my eye:

1. Lack of documentation. To me it is surprising, that you went through the trouble of documenting a class that is an event argument, but completely left out IAutoComplete interface, which plays a pivotal role in this entire thing. I can make an educated guess on what most of the members do but by no means it is obvious.

2. config.UserSettings.AutoCompleteSettings.AutoCompletes.FirstOrDefault - here you can use dictionary instead of doing a linear lookup.

3. autoComplete.IsEnabled = setting.IsEnabled; - you might want to consider replacing it with autoComplete.ApplySettings(setting). Even though it "leaks" settings class to the interface, it makes a job of extending and/or customizing settings much easier.

4. Personally, I prefer to split bool Execute into void Execute and bool CanExecute (what Microsoft does in ICommand interface). I think it separates concerns better and allows to override those methods individually. It also makes it clear, that bool is not the result of operation (success/failure) but an indication whether or not operation can be executed in the first place (can execute/cannot execute).

5. What pissed me off about Resharper is how hard it was to just input a single {. Without the }. And how it then removes both braces when you try to remove the second one... AFAIR they did something about it in later versions, but I am not sure what (maybe I just got used to it). You seem to mimic this behavior but I would love to see it improved in some way. :) Maybe add a setting, that can toggle this feature (the delete part) on or off.

6. Another thought about DeleteAroundCaret: maybe DELETE should be handled internally by autocomplete classes? I mean if autocomplete is responsible for adding "output", it should also know how to remove it. At least it makes sense to me.

7. VBENativeServices.KeyDown += HandleKeyDown; - I think you need another abstraction layer on top of this event, that would track which keys are used where. From my experience handling key presses in hotkey-heavy apps quickly becomes a nightmare, if you use a single event for that. You should come up with a way to manage hotkeys and detect conflicts. When multiple services want to handle, say, DELETE key in different way, there should be a clear policy in place, that should decide which service should handle this key in the current context, or throw an exception if the hotkey is ambiguous.

Like t3chb0t, I am not familiar with VBA, but I will mention a few things that caught my eye:

1. Lack of documentation. To me it is surprising, that you went through the trouble of documenting a class that is an event argument, but completely left out IAutoComplete interface, which plays a pivotal role in this entire thing. I can make an educated guess on what most of the members do but by no means it is obvious.

2. config.UserSettings.AutoCompleteSettings.AutoCompletes.FirstOrDefault - here you can use dictionary instead of doing a linear lookup.

3. autoComplete.IsEnabled = setting.IsEnabled; - you might want to consider replacing it with autoComplete.ApplySettings(setting). Even though it "leaks" settings class to the interface, it makes a job of extending and/or customizing settings much easier.

4. Personally, I prefer to split bool Execute into void Execute and bool CanExecute (what Microsoft does in ICommand interface). I think it separates concerns better and allows to override those methods individually. It also makes it clear, that bool is not the result of operation (success/failure) but an indication of whether or not operation can be executed in the first place (can execute/cannot execute).

5. What pissed me off about Resharper is how hard it was to just input a single {. Without the }. And how it then removes both braces when you try to remove the second one... AFAIR they did something about it in later versions, but I am not sure what (maybe I just got used to it). You seem to mimic this behavior but I would love to see it improved in some way. :) Maybe add a setting, that can toggle this feature (the delete part) on or off.

6. Another thought about DeleteAroundCaret: maybe DELETE should be handled internally by autocomplete classes? I mean if autocomplete is responsible for adding "output", it should also know how to remove it. At least it makes sense to me.

7. VBENativeServices.KeyDown += HandleKeyDown; - I think you need another abstraction layer on top of this event, that would track which keys are used where. From my experience handling key presses in hotkey-heavy apps quickly becomes a nightmare, if you use a single event for that. You should come up with a way to manage hotkeys and detect conflicts. When multiple services want to handle, say, DELETE key in different way, there should be a clear policy in place, that should decide which service should handle this key in the current context, or throw an exception if the hotkey is ambiguous.

Like t3chb0t, I am not familiar with VBA, but I will mention a few things that caught my eye:

1. Lack of documentation. To me it is surprising, that you went through the trouble of documenting a class that is an event argument, but completely left out IAutoComplete interface, which plays a pivotal role in this entire thing. I can make an educated guess on what most of the members do but by no means it is obvious.

2. config.UserSettings.AutoCompleteSettings.AutoCompletes.FirstOrDefault - here you can use dictionary instead of doing a linear lookup.

3. autoComplete.IsEnabled = setting.IsEnabled; - you might want to consider replacing it with autoComplete.ApplySettings(setting). Even though it "leaks" settings class to the interface, it makes a job of extending and/or customizing settings much easier.

4. Personally, I prefer to split bool Execute into void Execute and bool CanExecute (what Microsoft does in ICommand interface). I think it separates concerns better and allows to override those methods individually. It also makes it clear, that bool is not the result of operation (success/failure) but an indication whether or not operation can be executed in the first place (can execute/cannot execute).

5. What pissed me off about Resharper is how hard it was to just input a single {. Without the }. And how it then removes both braces when you try to remove the second one... AFAIR they did something about it in later versions, but I am not sure what (maybe I just get used to it). You seem to mimic this behavior but I would love to see it improved in some way. :) Maybe add a setting, that can toggle this feature on or off.

6. Another thought about DeleteAroundCaret: maybe DELETE should be handled internally by autocomplete classes? I mean if autocomplete is responsible for adding "output", it should also know how to remove it. At least it makes sense to me.

7. VBENativeServices.KeyDown += HandleKeyDown; - I think you need another abstraction layer on top of this event, that would track which keys are used where. From my experience handling key presses in hotkey-heavy apps quickly becomes a nightmare, if you use a single event for that. You should come up with a way to manage hotkeys and detect conflicts. When multiple services want to handle, say, DELETE key in different way, there should be a clear policy in place, that should decide which service should handle this key in the current context, or throw an exception if the hotkey is ambiguous.

Like t3chb0t, I am not familiar with VBA, but I will mention a few things that caught my eye:

1. Lack of documentation. To me it is surprising, that you went through the trouble of documenting a class that is an event argument, but completely left out IAutoComplete interface, which plays a pivotal role in this entire thing. I can make an educated guess on what most of the members do but by no means it is obvious.

2. config.UserSettings.AutoCompleteSettings.AutoCompletes.FirstOrDefault - here you can use dictionary instead of doing a linear lookup.

3. autoComplete.IsEnabled = setting.IsEnabled; - you might want to consider replacing it with autoComplete.ApplySettings(setting). Even though it "leaks" settings class to the interface, it makes a job of extending and/or customizing settings much easier.

4. Personally, I prefer to split bool Execute into void Execute and bool CanExecute (what Microsoft does in ICommand interface). I think it separates concerns better and allows to override those methods individually. It also makes it clear, that bool is not the result of operation (success/failure) but an indication whether or not operation can be executed in the first place (can execute/cannot execute).

5. What pissed me off about Resharper is how hard it was to just input a single {. Without the }. And how it then removes both braces when you try to remove the second one... AFAIR they did something about it in later versions, but I am not sure what (maybe I just got used to it). You seem to mimic this behavior but I would love to see it improved in some way. :) Maybe add a setting, that can toggle this feature (the delete part) on or off.

6. Another thought about DeleteAroundCaret: maybe DELETE should be handled internally by autocomplete classes? I mean if autocomplete is responsible for adding "output", it should also know how to remove it. At least it makes sense to me.

7. VBENativeServices.KeyDown += HandleKeyDown; - I think you need another abstraction layer on top of this event, that would track which keys are used where. From my experience handling key presses in hotkey-heavy apps quickly becomes a nightmare, if you use a single event for that. You should come up with a way to manage hotkeys and detect conflicts. When multiple services want to handle, say, DELETE key in different way, there should be a clear policy in place, that should decide which service should handle this key in the current context, or throw an exception if the hotkey is ambiguous.