principlecsharpMinor
Refactor (DRY, KISS) QueryStatus event for VS Addin
Viewed 0 times
kissquerystatusaddinfordryrefactorevent
Problem
I have complex logic in QueryStatus event (CommandTarget) for a VS Addin (VS 2010).
I'm a newbie using C#.
I would like elegant, easily maintainable code that follows DRY and KISS principles. Maybe using helper with ActionT> or FuncT> methods.
There are many commands to use: using
Main method:
comandosOracleNET method determines if a option in Menu is visible or not.
Main method:
```
private void comandosOracleNET(string CmdName, vsCommandStatusTextWanted NeededText, ref vsCommandStatus StatusOption, ref object CommandText)
{
EnvDTE80.Solution2 solucion = (EnvDTE80.Solution2)_applicationObject.Solution;
string etiqueta = null;
bool bajoSC = false;
if (_applicationObject.SourceControl != null
&& _applicationObject.Solution != null
&& _applicationObject.Solution.IsOpen
&& !solucion.FullName.IsNullOrWhiteSpace()
&& _applicationObject.SourceControl.IsItemUnderSCC(solucion.FullName))
{
etiqueta = Logica.OperacionesEtiquetado.ObtenerEtiquetaSolucion(solucion);
bajoSC = true;
}
#region Branch y Merge
if (CmdName == _addInInstance.ProgID + "." + ComandoBranchItemSqlForSW_Na
I'm a newbie using C#.
I would like elegant, easily maintainable code that follows DRY and KISS principles. Maybe using helper with ActionT> or FuncT> methods.
There are many commands to use: using
if statements and switch statements.Main method:
QueryStatus event call to comandosOracleNET method.comandosOracleNET method determines if a option in Menu is visible or not.
void IDTCommandTarget.QueryStatus(string CmdName, vsCommandStatusTextWanted NeededText, ref vsCommandStatus StatusOption, ref object CommandText)
{
if (!OnStartupCompleted) return;
try
{
if (NeededText == vsCommandStatusTextWanted.vsCommandStatusTextWantedNone)
{
comandosOracleNET(CmdName, NeededText, ref StatusOption, ref CommandText);
return;
}
}
catch (Exception exc)
{
ErroresManager.TratamientoExcepcionEnConnectQueryStatus(exc, "Error en Command. Excepción en IDTCommandTarget.QueryStatus.");
}
}Main method:
```
private void comandosOracleNET(string CmdName, vsCommandStatusTextWanted NeededText, ref vsCommandStatus StatusOption, ref object CommandText)
{
EnvDTE80.Solution2 solucion = (EnvDTE80.Solution2)_applicationObject.Solution;
string etiqueta = null;
bool bajoSC = false;
if (_applicationObject.SourceControl != null
&& _applicationObject.Solution != null
&& _applicationObject.Solution.IsOpen
&& !solucion.FullName.IsNullOrWhiteSpace()
&& _applicationObject.SourceControl.IsItemUnderSCC(solucion.FullName))
{
etiqueta = Logica.OperacionesEtiquetado.ObtenerEtiquetaSolucion(solucion);
bajoSC = true;
}
#region Branch y Merge
if (CmdName == _addInInstance.ProgID + "." + ComandoBranchItemSqlForSW_Na
Solution
Using
You have to much vertical spacing (new lines) in your code so Sam the Maintainer can't grasp the code at first glance.
Using braces
Using the english language for naming variables will help Sam the Maintainer to maintain the code if he/she doesn't speak your language.
While we are at the naming of variables, based on the naming guidelines input parameters should be named using
A
makes the
By using a second guard clause in the
The
By extracting the if statements of
now the former 2
and by extracting the inner
it can be further reduced to
In a similiar way you can extract more of your code to separate methods.
case ComandoDespliegueSqlServer:
This case can be easily extracted to a improved method like
leaving the
#region in a method is a very strong indicator that the method should be broken into smaller methods. It is considered as code smell.You have to much vertical spacing (new lines) in your code so Sam the Maintainer can't grasp the code at first glance.
Using braces
{} for single if statements is a good start to make your code less error prone. If you consider to not use them, you should stick to your style you choose. Right now you are mixing them. Using the english language for naming variables will help Sam the Maintainer to maintain the code if he/she doesn't speak your language.
While we are at the naming of variables, based on the naming guidelines input parameters should be named using
camelCase casing. A
if..else like if (condition)
{
// some code
return;
}
else
{
//some other code
}makes the
else part redundant, you can simplify this to if (condition)
{
// some code
return;
}
//some other codeBy using a second guard clause in the
IDTCommandTarget.QueryStatus() method, you can reduce the horizintal spacing which add readability to your code. The
return statement in the try..catch is redundant and should be removed. void IDTCommandTarget.QueryStatus(string cmdName, vsCommandStatusTextWanted neededText, ref vsCommandStatus statusOption, ref object commandText)
{
if (!OnStartupCompleted) { return; }
if (neededText != vsCommandStatusTextWanted.vsCommandStatusTextWantedNone) { return; }
try
{
comandosOracleNET(cmdName, neededText, ref statusOption, ref commandText);
}
catch (Exception exc)
{
ErroresManager.TratamientoExcepcionEnConnectQueryStatus(exc, "Error en Command. Excepción en IDTCommandTarget.QueryStatus.");
}
}By extracting the if statements of
#region Branch y Merge to a separate method you can reduce code duplication. private bool IsBrancheOrMerge(String cmdName)
{
return (cmdName == _addInInstance.ProgID + "." + ComandoBranchItemSqlForSW_Name) ||
(cmdName == _addInInstance.ProgID + "." + ComandoMergeItemSqlForSW_Name);
}now the former 2
if statements can be reduced to if(IsBrancheOrMerge(cmdName))
{
if (_applicationObject.SelectedItems.Count == 1 && _applicationObject.SelectedItems.Item(1).ProjectItem.FileCount == 1)
{
string aux = _applicationObject.SelectedItems.Item(1).ProjectItem.get_FileNames(1);
if (aux.ToLower().EndsWith(".sql"))
{
StatusOption = vsCommandStatus.vsCommandStatusSupported | vsCommandStatus.vsCommandStatusEnabled;
}
else
{
StatusOption |= vsCommandStatus.vsCommandStatusInvisible;
}
}
return;
}and by extracting the inner
if statements to a method ProcessBranchOrMerge() private void ProcessBranchOrMerge(ref vsCommandStatus statusOption)
{
if (_applicationObject.SelectedItems.Count == 1 && _applicationObject.SelectedItems.Item(1).ProjectItem.FileCount == 1)
{
string aux = _applicationObject.SelectedItems.Item(1).ProjectItem.get_FileNames(1);
if (aux.ToLower().EndsWith(".sql"))
{
statusOption = vsCommandStatus.vsCommandStatusSupported | vsCommandStatus.vsCommandStatusEnabled;
}
else
{
statusOption |= vsCommandStatus.vsCommandStatusInvisible;
}
}
}it can be further reduced to
if(IsBrancheOrMerge(cmdName))
{
ProcessBranchOrMerge(ref statusOption);
return;
}In a similiar way you can extract more of your code to separate methods.
case ComandoDespliegueSqlServer:
This case can be easily extracted to a improved method like
private void ProcessComandoDespliegueSqlServer(ref vsCommandStatus statusOption, EnvDTE80.Solution2 solucion)
{
statusOption = vsCommandStatus.vsCommandStatusSupported | vsCommandStatus.vsCommandStatusInvisible;
if (!solucion.IsOpen) { return; }
object[] proyectos = (object[])_applicationObject.ActiveSolutionProjects;
if (proyectos.Length != 1) { return; }
if (Logica.Operaciones.IsDeploySqlServerProject((Project)proyectos[0]))
{
statusOption |= vsCommandStatus.vsCommandStatusEnabled;
}
}leaving the
case case ComandoDespliegueSqlServer:
ProcessComandoDespliegueSqlServer(ref statusOption, solucion);
return;Code Snippets
if (condition)
{
// some code
return;
}
else
{
//some other code
}if (condition)
{
// some code
return;
}
//some other codevoid IDTCommandTarget.QueryStatus(string cmdName, vsCommandStatusTextWanted neededText, ref vsCommandStatus statusOption, ref object commandText)
{
if (!OnStartupCompleted) { return; }
if (neededText != vsCommandStatusTextWanted.vsCommandStatusTextWantedNone) { return; }
try
{
comandosOracleNET(cmdName, neededText, ref statusOption, ref commandText);
}
catch (Exception exc)
{
ErroresManager.TratamientoExcepcionEnConnectQueryStatus(exc, "Error en Command. Excepción en IDTCommandTarget.QueryStatus.");
}
}private bool IsBrancheOrMerge(String cmdName)
{
return (cmdName == _addInInstance.ProgID + "." + ComandoBranchItemSqlForSW_Name) ||
(cmdName == _addInInstance.ProgID + "." + ComandoMergeItemSqlForSW_Name);
}if(IsBrancheOrMerge(cmdName))
{
if (_applicationObject.SelectedItems.Count == 1 && _applicationObject.SelectedItems.Item(1).ProjectItem.FileCount == 1)
{
string aux = _applicationObject.SelectedItems.Item(1).ProjectItem.get_FileNames(1);
if (aux.ToLower().EndsWith(".sql"))
{
StatusOption = vsCommandStatus.vsCommandStatusSupported | vsCommandStatus.vsCommandStatusEnabled;
}
else
{
StatusOption |= vsCommandStatus.vsCommandStatusInvisible;
}
}
return;
}Context
StackExchange Code Review Q#71296, answer score: 5
Revisions (0)
No revisions yet.