Not knowing the rest of your code or the exact contents it is difficult to offer better advice, but having multiple ifs isn't necessarily a bad thing as long as the code is readable, its not repetitive (each if does distinctly different things), and it does what is expected. But as you seem to always want to return you can add that last, if no Error then normal importModel will be returned.
if (fileCSV == null && fileCSV.ContentLength == 0)
{
importModel.Error = "Error1";
}
else
{
List<ImportModel> mappings = _importService.ImportCSVToList<ImportModel>(fileCSV);
if (mappings.Count > 0)
{
IEnumerable<ImportModel> duplicates = mappings.GroupBy(x => x.ProductSku).SelectMany(g => g.Skip(1)).ToList();
if (duplicates.Any())
{
importModel.Error = "Error2";
}
else
{
var products = _productService.GetProducts(productSkuList).ToList();
if (!importModel.InvalidSkuList.Any())
{
bool isImported = _productService.Import(mappings);
if (!isImported)
{
importModel.Error = "Error3";
}
}
}
}
else
{
importModel.Error = "Error4";
}
}
return importModel;
If you really want to shorten the code you could remove many of the else because you have return in the ifs
if (fileCSV == null && fileCSV.ContentLength == 0)
{
importModel.Error = "Error1";
return importModel;
}
List<ImportModel> mappings = _importService.ImportCSVToList<ImportModel>(fileCSV);
if (mappings.Count > 0)
{
IEnumerable<ImportModel> duplicates = mappings.GroupBy(x => x.ProductSku).SelectMany(g => g.Skip(1)).ToList();
if (duplicates.Any())
{
importModel.Error = "Error2";
return importModel;
}
var products = _productService.GetProducts(productSkuList).ToList();
if (!importModel.InvalidSkuList.Any())
{
bool isImported = _productService.Import(mappings);
if (!isImported)
{
importModel.Error = "Error3";
return importModel; // added, though depending on your intentions the logic here could differ
}
// should there be a separate Error3.5 here?
}
return importModel;
}
importModel.Error = "Error4";
return importModel;