Skip to main content
deleted 82 characters in body
Source Link
depperm
  • 1.1k
  • 6
  • 14

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 the code overall 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";
                }
                // should there be a separate Error here or only if !isImported
            }
        }
    }
    else
    {
        importModel.Error =  "Error4";
    }
}
return importModel;

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 the code overall 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";
                }
                // should there be a separate Error here or only if !isImported
            }
        }
    }
    else
    {
        importModel.Error =  "Error4";
    }
}
return importModel;

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 the code overall 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
        {
            if (!importModel.InvalidSkuList.Any())
            {
                bool isImported = _productService.Import(mappings);
                if (!isImported)
                {
                    importModel.Error = "Error3";
                }
                // should there be a separate Error here or only if !isImported
            }
        }
    }
    else
    {
        importModel.Error =  "Error4";
    }
}
return importModel;
added 16 characters in body
Source Link
depperm
  • 1.1k
  • 6
  • 14

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 ifif does distinctly different things), and itthe code overall 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";
                }
                // should there be a separate Error here or only if !isImported
            }
        }
    }
    else
    {
        importModel.Error =  "Error4";
    }
}
return importModel;

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";
                }
                // should there be a separate Error here or only if !isImported
            }
        }
    }
    else
    {
        importModel.Error =  "Error4";
    }
}
return importModel;

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 the code overall 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";
                }
                // should there be a separate Error here or only if !isImported
            }
        }
    }
    else
    {
        importModel.Error =  "Error4";
    }
}
return importModel;
deleted 1029 characters in body
Source Link
depperm
  • 1.1k
  • 6
  • 14
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// ==should 0)
{
there be a separate 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();
 here or only if (duplicates.Any())!isImported
    {
        importModel.Error = "Error2";}
        return importModel;
    }
    var products = _productService.GetProducts(productSkuList).ToList();}
    if (!importModel.InvalidSkuList.Any())else
    {
        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?"Error4";
    }
    return importModel;
}
importModel.Error =  "Error4";
return importModel;
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;
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";
                }
                // should there be a separate Error here or only if !isImported
            }
        }
    }
    else
    {
        importModel.Error =  "Error4";
    }
}
return importModel;
Source Link
depperm
  • 1.1k
  • 6
  • 14
Loading