2
\$\begingroup\$

I have a helper extension method that I use for path string validation. It looks as below. It's purpose is to work with network paths as well, so in case a network resource is not available, I do not want my applications to freeze, hence the specified timeout etc.

public static bool IsExistingFilePath(this string source, int numberOfAttempts = 3, int timeout = 300)
{
    for (int i = 0; i < numberOfAttempts; i++)
    {

        var task = new Task<bool>(() => File.Exists(source));
        task.Start();

        bool exists = task.Wait(timeout) && task.Result;
        if (exists)
        {
            return true;
        }
        else
            continue;
    }
    return false;
}

I also have a generic text box control, where I want to validate the provided path - and indicate if path is not correct. I want this auto text box validation to happen as infrequently as possible, because I don't want to run this loop of File.Exists() tasks when the path is being provided at the moment (e.g. user typing) etc.

My current approach is that I have a property that is bound to textbox text, and whenever it is updated, the validation is performed (update source trigger = property changed, mode = two way).

However, there is a problem, because property setter is not accessed when I paste into the text box nor when I write in it. I had set up a bunch of event handlers (OnPaste and LostFocus), but it's becoming unmanageable and appears not correct - and also appears to fire more times then necessary.

Then I thought about data validation from XAML with a ValidationRule class, but that actually works more or less like with KeyUp.

What would be the correct way to perform validation only when needed?

  • When text is pasted into box
  • When property is changed for whatever reason
  • When textbox is cleared
  • When text is changed (for instance, a proper path is pasted and a user decides to change it manually, e.g. use different drive)
\$\endgroup\$
  • 1
    \$\begingroup\$ Please don't edit new code into questions. It invalidates answers. If you would like to post a new, on-topic question, you may do so. \$\endgroup\$ – SirPython Dec 3 '15 at 0:01
  • \$\begingroup\$ Why use a text box at all when there are File & Folder picker controls? \$\endgroup\$ – RubberDuck Dec 3 '15 at 2:22
  • \$\begingroup\$ For many reasons - you want to display the current path, you want to enable editing it (e.g. in a fixed folder structure you can change current path by editing the string directly instead of picking the path again), you might want to allow pasting the path or dragging and dropping files to resolve the path, or dragging selected texts. It's all so much quicker than picker controls if you know your way around computers - and my users do:) \$\endgroup\$ – Bartosz Dec 3 '15 at 9:37
2
\$\begingroup\$

Start a Timer.

Whenever the text box changes, the user presses a key, the TextChanged event occurs, etc, start the timer. If it changes again, reset it. Create some timeout that, after which, the file validation would occur. (200ms would be plenty)

This has the obvious benefit of reducing the number of IO requests you do, and it helps keep responsiveness up. You can also make it asynchronous, and only hold the UI when doing actual updating.

As far as your code goes...

I would add braces in your else condition. I used to omit braces whenever possible, but I encountered more than one bug as a result, and it's just as easy to add them.

There's really not much else to comment on; there's not a lot going on here and it's quite clean to begin with.

\$\endgroup\$
  • \$\begingroup\$ Thanks a lot - so you would suggest to start the timer on the most typical/frequent event, like KeyUp? \$\endgroup\$ – Bartosz Dec 2 '15 at 23:41
  • \$\begingroup\$ @Bartosz Yeah, or KeyDown, or KeyPress. Whatever event you like. :) (Or, better yet, TextChanged. This will cover all scenarios.) \$\endgroup\$ – Der Kommissar Dec 2 '15 at 23:41
  • \$\begingroup\$ Actually, I am getting an invalid operation exception when trying to do that ` The calling thread cannot access this object because a different thread owns it.` My code is in the edited question. \$\endgroup\$ – Bartosz Dec 2 '15 at 23:57
  • \$\begingroup\$ @Bartosz You should visit Stack Overflow for that issue. The original scope of the question did not include it. Feel free to link me if you do post a question there. \$\endgroup\$ – Der Kommissar Dec 3 '15 at 0:05
  • \$\begingroup\$ OK, fair point! \$\endgroup\$ – Bartosz Dec 3 '15 at 0:10
2
\$\begingroup\$

Regarding your IsExistingFilePath

  1. Starting a task and the immediately block (task.Result) is a huge antipattern. All the downsides and no benefit, the calling thread is freezing your UI now. Also there is overhead in creating running a task so it will block even longer. By doing this you open up the door for deadlocks and race conditions, all kinds of pain.
  2. When you start a task prefer Task.Run(() => ...) it is shorter and you can't forget to call Start().
  3. Normally you don't want to start a task to block for IO.
  4. If you use a task you should do async all the way.
\$\endgroup\$
  • \$\begingroup\$ Thanks for your feedback - would you mind showing how you would improve the code, please? I'd be very grateful. Please note that the purpose of this method is following - I have noticed that in some cases when the path is a network path, if the file does not exist or network is not available, getting response takes a lot of time (haven't measured it, but I'd say that more than a minute) - whereas when the task exists and is available, response is a matter of <100ms. Therefore, this method makes sure that the UI is not really blocked for more than 3x100ms or something. \$\endgroup\$ – Bartosz Feb 23 '16 at 20:32
  • \$\begingroup\$ (cont.) because we only wait a very short time for the response. Hope this makes sense... Point 2 seems indeed a fair point as a best practice ;) \$\endgroup\$ – Bartosz Feb 23 '16 at 20:35
1
\$\begingroup\$

let me post the 'more or less' complete solution to the problem that I applied based on EBrown's answer (thanks). It appears to work pretty well, hope it helps someone in the future.

//timer that will control how often is the path validation logic executed
private readonly System.Timers.Timer _pathValidationTimer = new System.Timers.Timer(250) { AutoReset = false };

    //a constructor  of my user control with path validation logic
    public PathBox2()
    {
        InitializeComponent();

        _pathValidationTimer.Elapsed += (timerSender, args) =>
        {
            Dispatcher.BeginInvoke((Action)delegate
            {
                CheckPath();
                _pathValidationTimer.Stop();

            });
        };
    }


    private void PathBox_OnTextChanged(object sender, TextChangedEventArgs e)
    {
        //restarting the timer
        _pathValidationTimer.Stop();
        _pathValidationTimer.Start();

    }

    //the path check handler
    public bool CheckPath()
    {
        if (!CheckPaths)
            return true;
        //the control can be either a file path box or a folder box
        bool exists = IsFolderBox ? ProvidedPath.IsExistingDirPath(numberOfAttempts:1) : ProvidedPath.IsExistingFilePath(numberOfAttempts:1);

        if (!exists)
        {
            ConfirmationSignColorBrush = Brushes.DarkRed;
            ConfirmationTickLabel = "✘";
            return false;
        }
        else
        {
            UpdatePreviousPathsList(ProvidedPath); //it's a combobox with previous paths stored
            ConfirmationSignColorBrush = Brushes.Green;
            ConfirmationTickLabel = "✔";
            return true;
        }
    }

    //complete extension method for path validation
    public static bool IsExistingFilePath(this string source, bool showMsgIfMissing = false, int numberOfAttempts = 3, int timeout = 300, bool networkSafe = true)
    {
        if (networkSafe)
        {
            for (int i = 0; i < numberOfAttempts; i++)
            {
                var task = new Task<bool>(() => File.Exists(source));
                task.Start();
                bool exists = task.Wait(timeout) && task.Result;
                if (exists)
                {
                    return true;
                }
            }
            if (showMsgIfMissing)
            {
                Msg.Show(string.Format("Cannot find file:\n{0}", string.IsNullOrWhiteSpace(source) ? "Path is empty" : source), "File not available", MsgBtn.OK, MsgImg.Warning);
            }
            return false;
        }
        else
        {
            if (File.Exists(source))
            {
                return true;
            }
            else
            {
                if (showMsgIfMissing)
                {
                    Msg.Show(string.Format("Cannot find file:\n{0}", source), "File not available", MsgBtn.OK, MsgImg.Warning);
                }
                return false;
            }
        }
    }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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