C# Refactor Switch-Anweisung mit Null-Check

Question

Ich habe nur ein bisschen Probleme mit ein paar Switch-Anweisungen wirklich und ich fühle, es gibt einen viel besseren Weg zum Erreichen des Endziels.

Im Wesentlichen bin ich im Viewmodel in eine Methode übergeben. Die Methode ruft zunächst das Objekt ab, auf das sich das Viewmodel bezieht, aus der Datenbank, dann führt eine switch-Anweisung eine Nullprüfung für eine bestimmte Eigenschaft durch. Auf der Grundlage dieses Ergebnisses führt eine andere switch-Anweisung eine weitere Nullprüfung des Ansichtsmodells durch. An jedem Punkt werden dem Objekt Werte aus der Datenbank zugewiesen, dann findet am Ende eine Datenbankaktualisierung statt.

Hier ist der Code

 public async Task UpdateContractWithRepository(ViewModel viewModel) 
    { 
     // Get the contract from db 
     Contract contract = GetContract(viewModel.Id); 

     switch (viewModel.RepositoryId == null) 
     { 
      case true: 
       switch (contract.RepositoryId == null) 
       { 
        case true: 
         // nothing to do 
         // no change 
         break; 
        case false: 
         // Unassign Repository 
         UpdateRepositoryAssignment(contract.RepositoryId, false); 

         // Update properties 
         contract.RepositoryId = null; 
         contract.ConsumedUnits = null; 
         break; 
       } 
       break; 
      case false: 
       switch (contract.RepositoryId == null) 
       { 
        case true: 
         // assign repository 
         UpdateRepositoryAssignment(viewModel.RepositoryId, true); 

         // Get repository 
         Repository repository = GetRepository(viewModel.RepositoryId); 

         // Update properties 
         contract.RepositoryId = repository.Id; 
         contract.ConsumedUnits = repository.Units; 
         break; 
        case false: 
         // assign repository 
         UpdateRepositoryAssignment(viewModel.RepositoryId, true); 

         // Get repository 
         Repository repository = GetRepository(viewModel.RepositoryId); 

         // Update properties 
         contract.RepositoryId = repository.Id; 
         contract.ConsumedUnits = repository.Units; 
         break; 
       } 
       break; 

     } 

     UpdateContract(contract); 
    } 

Ich denke, ich wahrscheinlich mit den Switch-Anweisungen tun könnte, weg und verwenden, wenn Aussagen statt, gibt es noch einige Verschachtelung seien von dem, was ich sagen kann. Ich fragte mich nur, ob jemand irgendwelche Vorschläge hatte.

Ich habe Refactoring Switch-Anweisungen hier betrachtet, aber sie scheinen nicht wirklich Null-Checks abzudecken.

Jede Hilfe wird geschätzt!

Answer 1

Der gesamte Code vereinfacht werden kann, wenn man nur zwei bools nehmen:

bool IsVMRepoNull = viewModel.RepositoryId == null; 
bool IsContractRepoNull = contract.RepositoryId == null; 

if(IsVMRepoNull && !IsContractRepoNull) 
{ 
    UpdateRepositoryAssignment(contract.RepositoryId, false); 

// Update properties 
    contract.RepositoryId = null; 
    contract.ConsumedUnits = null; 
} 
else if(!IsVMRepoNull) 
{ 
    UpdateRepositoryAssignment(viewModel.RepositoryId, true); 

    // Get repository 
    Repository repository = GetRepository(viewModel.RepositoryId); 

    // Update properties 
    contract.RepositoryId = repository.Id; 
    contract.ConsumedUnits = repository.Units; 
} 

Answer 2

Ich bin nicht ganz sicher, wo Sie stecken geblieben sind - aber die Schalter mit if/else ersetzt ist fast so einfach wie switch mit if ersetzen. Hier ist der transformierte Code:

public async Task UpdateContractWithRepository(ViewModel viewModel) 
{ 
    // Get the contract from db 
    Contract contract = GetContract(viewModel.Id); 

    if (viewModel.RepositoryId == null) 
    { 
     if (contract.RepositoryId == null) 
     { 
      // nothing to do 
      // no change 
     } else { 
      // Unassign Repository 
      UpdateRepositoryAssignment(contract.RepositoryId, false); 

      // Update properties 
      contract.RepositoryId = null; 
      contract.ConsumedUnits = null; 
     } 
    } else { 
     if (contract.RepositoryId == null) 
     { 
      // assign repository 
      UpdateRepositoryAssignment(viewModel.RepositoryId, true); 

      // Get repository 
      Repository repository = GetRepository(viewModel.RepositoryId); 

      // Update properties 
      contract.RepositoryId = repository.Id; 
      contract.ConsumedUnits = repository.Units; 
     } else { 

      // assign repository 
      UpdateRepositoryAssignment(viewModel.RepositoryId, true); 

      // Get repository 
      Repository repository = GetRepository(viewModel.RepositoryId); 

      // Update properties 
      contract.RepositoryId = repository.Id; 
      contract.ConsumedUnits = repository.Units; 
     } 
    } 

    UpdateContract(contract); 
} 

Answer 3

Ich denke, dass es nicht notwendig ist, es so kompliziert zu machen. testen Werde ich denke, dass es gleiche Arbeit zu tun:

public async Task UpdateContractWithRepository(ViewModel viewModel){ 
     Contract contract = GetContract(viewModel.Id); 

     if (viewModel.RepositoryId == null) 
     { 
      if(contract.RepositoryId != null){ 
       UpdateRepositoryAssignment(contract.RepositoryId, false); 
       contract.RepositoryId = null; 
       contract.ConsumedUnits = null; 
      } 
     } 
     else 
     { 
      if (contract.RepositoryId == null) 
      { 
       UpdateRepositoryAssignment(viewModel.RepositoryId, true); 
       Repository repository = GetRepository(viewModel.RepositoryId); 
       contract.RepositoryId = repository.Id; 
       contract.ConsumedUnits = repository.Units; 
      } 
      else 
      { 
       UpdateRepositoryAssignment(viewModel.RepositoryId, true); 
       Repository repository = GetRepository(viewModel.RepositoryId); 
       contract.RepositoryId = repository.Id; 
       contract.ConsumedUnits = repository.Units; 
      } 
     } 
     UpdateContract(contract); 
    } 

Answer 4

ich wirklich if....else if mit Bool Bedingungen verwenden würde. Sie können es besser lesbar mit aussagekräftigen variabler Namen machen:

public async Task UpdateContractWithRepository(ViewModel viewModel) 
{ 
    // Get the contract from db 
    Contract contract = GetContract(viewModel.Id); 

    bool modelRepositoryKnown = viewModel.RepositoryId != null; 
    bool contractRepositoryKnown = contract.RepositoryId != null; 

    if (modelRepositoryKnown) 
    { 
     if (contractRepositoryKnown) 
     { 
      // assign repository 
      UpdateRepositoryAssignment(viewModel.RepositoryId, true); 

      // Get repository 
      Repository repository = GetRepository(viewModel.RepositoryId); 

      // Update properties 
      contract.RepositoryId = repository.Id; 
      contract.ConsumedUnits = repository.Units; 
     } 
     else 
     { 
      // assign repository 
      UpdateRepositoryAssignment(viewModel.RepositoryId, true); 

      // Get repository 
      Repository repository = GetRepository(viewModel.RepositoryId); 

      // Update properties 
      contract.RepositoryId = repository.Id; 
      contract.ConsumedUnits = repository.Units; 
     } 
    } 
    else if(contractRepositoryKnown) // Model-Repository unknown but Contract-Repository Known 
    { 
     // Unassign Repository 
     UpdateRepositoryAssignment(contract.RepositoryId, false); 

     // Update properties 
     contract.RepositoryId = null; 
     contract.ConsumedUnits = null; 
     break; 
    } 
    UpdateContract(contract); 
} 

Answer 5

Sie es erreichen können verschachtelte Schaltzustände durch die Kombination von in eine if else-Anweisung, die mehrere Bedingungen verwendet.

if(viewModel.RepositoryId == null && !contract.RepositoryId) 
{ 
    // Unassign Repository 
    UpdateRepositoryAssignment(contract.RepositoryId, false); 
    // Update properties 
    contract.RepositoryId = null; 
    contract.ConsumedUnits = null; 
} 
else if(!viewModel.RepositryId && contract.RepositoryId == null) 
{ 
    // assign repository 
    UpdateRepositoryAssignment(viewModel.RepositoryId, true); 
    // Get repository 
    Repository repository = GetRepository(viewModel.RepositoryId); 
    // Update properties 
    contract.RepositoryId = repository.Id; 
    contract.ConsumedUnits = repository.Units; 
} 
else if(!viewModel.RepositryId && !contract.RepositoryId == null) 
{ 
    // assign repository 
    UpdateRepositoryAssignment(viewModel.RepositoryId, true); 
    // Get repository 
    Repository repository = GetRepository(viewModel.RepositoryId); 
    // Update properties 
    contract.RepositoryId = repository.Id; 
    contract.ConsumedUnits = repository.Units; 
}