1
\$\begingroup\$

does anyone knows how I can simplify this code?

The objective is to get multiple files from the explorer and validate the number of files, the weight of each file and the total weight.

public static async Task<IEnumerable<StorageFile>> GetMultipleFileFromDisk(Window rootWindow, int maxFiles,
            string fileTypes = null, ulong fileMaxSize = 0, ulong totalMaxSize = 0,
            PickerLocationId? pickerLocation = null, PickerViewMode viewMode = PickerViewMode.List)
        {
            var filePicker = new FileOpenPicker();
            if (fileTypes != null)
            {
                var fileTypesArray = fileTypes.Split(";");
                //IList<string> fileTypesList = fileTypes.Split(";");

                // Get the current window's Handler by passing in the Window object
                var handler = WinRT.Interop.WindowNative.GetWindowHandle(rootWindow);

                // Associate the HWND with the file picker
                WinRT.Interop.InitializeWithWindow.Initialize(filePicker, handler);

                if (fileTypesArray.Count() >= 0)
                    foreach (var fileType in fileTypesArray)
                        filePicker.FileTypeFilter.Add(fileType);
            }

            filePicker.SuggestedStartLocation = pickerLocation ?? PickerLocationId.DocumentsLibrary;
            filePicker.ViewMode = viewMode;

            var files = await filePicker.PickMultipleFilesAsync();
            if (files.Count > maxFiles)
                throw new Exception($"Só pode carregar {maxFiles} ficheiros.");

            IEnumerable<StorageFile> finalFiles;

            if (fileMaxSize != 0)
            {
                var correctFiles = new List<StorageFile>();
                foreach (var file in files)
                {
                   
                    var hasRightSize = await HasRightSize(file, fileMaxSize);
                    if (hasRightSize)
                        correctFiles.Add(file);
                    else
                        throw new Exception(file.Name + " excede o tamanho máximo.");
                }
                finalFiles = correctFiles;
            } else
                finalFiles = files;

            if (totalMaxSize <= 0) return finalFiles;
            if (!await HasRightSize(finalFiles, totalMaxSize))
                finalFiles = null;

            return finalFiles;
        }
\$\endgroup\$
1

1 Answer 1

1
\$\begingroup\$

Here are my refactoring ideas

The if(fileTypes != null) block

if (fileTypes != null)
{
    var handler = WinRT.Interop.WindowNative.GetWindowHandle(rootWindow);
    WinRT.Interop.InitializeWithWindow.Initialize(filePicker, handler);
        
    foreach (var fileType in fileTypes.Split(";"))
        filePicker.FileTypeFilter.Add(fileType);
}
  • You don't need to put the result of Split into a variable, like fileTypesArray
    • You can use it directly inside the foreach loop
  • You don't need to guard your foreach loop with an empty collection check
    • If it is empty then it will not execute the loop body
    • If the Split could return null (but it couldn't) then it would make sense to guard it with a null check

The if (fileMaxSize != 0) block

IEnumerable<StorageFile> finalFiles = files;
if (fileMaxSize != 0)
{
    var correctFiles = new List<StorageFile>();
    foreach (var file in files)
    {
        if (!await HasRightSize(file, fileMaxSize))
            throw new Exception(file.Name + " excede o tamanho máximo.");
        correctFiles.Add(file);
    }
    finalFiles = correctFiles;
}
  • You can omit the outer else branch by initializing the finalFiles with the files
  • You can omit the inner else branch by inverting the if condition since the throw statement breaks the execution
    • So, it will not reach the Add command if it does not have the right size

Based on your requirements it might make sense to use a more specific exception (either a built-in or a custom one).

The final return statements

if (totalMaxSize <= 0)
    return finalFiles;

return await HasRightSize(finalFiles, totalMaxSize) ? finalFiles : null;

Or

return totalMaxSize <= 0
        ? finalFiles
        : await HasRightSize(finalFiles, totalMaxSize)
            ? finalFiles
            : null;
  • You can take advantage of the ternary conditional operator to make your statements more concise
  • I depends on personal preference whether the latter option, with the nested conditional operator, is more legible than the former one
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.