8
\$\begingroup\$

I made a do...while loop in order to wait for the web browser to load completely in order to enter username and password. I don't know whether this impacts the system a lot because I repeated it in at least 4 places. Is there a better solution?

private async void DeleteProduct()
{

    if (string.IsNullOrEmpty(TextBoxFile.Text))
    {
        MessageBox.Show(
            "Vous n'avez pas entré l'emplacement du fichier!", "Attention", MessageBoxButton.OK,
            MessageBoxImage.Exclamation);
    }
    else
    {


    MainwebBrowser.Source = "http://www.easyrea.net/cart/detail".ToUri();
    do
    {
        await Task.Delay(500);
    } while (!MainwebBrowser.IsDocumentReady);

    MainwebBrowser.ExecuteJavascript(
        "document.getElementById('login_login').value = '" + Properties.Settings.Default.Username + "';");
    MainwebBrowser.ExecuteJavascript(
        "document.getElementById('login_password').value = '" + Properties.Settings.Default.Password + "';");
    MainwebBrowser.ExecuteJavascript("document.forms['loginForm'].submit()");
    do
    {
        await Task.Delay(500);
    } while (!MainwebBrowser.IsDocumentReady);
    DataTable dt = LoadXls(dlg.FileName);
        progressBar1.Value = 0.0;
        foreach (DataRow row in dt.Rows)
        {
            if (!_checkcancelled)
            {
                string text = "#product" + row["Référence"];
                string cmd =
                    MainwebBrowser.ExecuteJavascriptWithResult(
                        string.Format("$('{0}').find('.cmd').text();", text));
                string existed =
                    MainwebBrowser.ExecuteJavascriptWithResult(
                        string.Format("$('{0}').find('#quantity').text();", text));
                if (cmd != "" & existed != "")
                {
                    int intcmd;
                    bool isintcmd = int.TryParse(Regex.Match(cmd, @"\d+").Value, out intcmd);

                    int intexisted;
                    bool isintexisted = int.TryParse(existed, out intexisted);
                    if (isintcmd & isintexisted)
                    {
                        if (intcmd >= intexisted)
                        {
                            MainwebBrowser.ExecuteJavascriptWithResult(
                                string.Format(
                                    "updateCartProduct('{0}','0','/cart/update',-{1},'72','1',0.65, 1)",
                                    row["Référence"], intexisted));
                        }
                        else if (intcmd < intexisted)
                        {
                            int inttobesubed = intexisted - (intexisted - intcmd);
                            MainwebBrowser.ExecuteJavascriptWithResult(
                                string.Format(
                                    "updateCartProduct('{0}','0','/cart/update',-{1},'72','1',0.65, 1)",
                                    row["Référence"], inttobesubed));
                        }
                    }
                }
                progressBar1.Value++;


            }
            else
            {
                //  label4.Text = "Annulé...";
                await Task.Delay(2000);
                // label4.Text = "Libre";
                deleteProductSuccess = false;
                CommanderButtonImport.IsEnabled = true;
                break;
            }
            deleteProductSuccess = true;
            await Task.Delay(1000);
        }
        if (!deleteProductSuccess)
        {
            progressBar1.Value = 0;
            EnableControl();
            CommanderButtonImport.IsEnabled = true;
            //  button5.Visible = false;
            //  label4.Text = "Annulé...";
            await Task.Delay(2000);
            //   label4.Text = "Libre";
        }
        else
        {
            progressBar1.Value = 0;
            EnableControl();
            CommanderButtonImport.IsEnabled = true;
            MainwebBrowser.Reload(false);
            await Task.Delay(5000);
                ModernDialog1.ShowMessage("L'Opération est terminée. "+Environment.NewLine+"Voulez-vous ouvrir la fichier Excel pour les produits manquant?",
                    "Terminée", MessageBoxButton.YesNo);
        }




    }
}
\$\endgroup\$
  • \$\begingroup\$ Great! I've voted to reopen this question and edited your post a bit, but to get the best out of Code Review you would include more plain-text details about what the code is doing. Don't worry about your English, we can help you with that =) \$\endgroup\$ – Mathieu Guindon Jul 8 '15 at 21:28
  • \$\begingroup\$ The French in the last message is a bit off: "L'opération", "\n", "Voulez-vous", "le fichier Excel", " ?" The incorrect newline, in particular, suggests that you haven't tested this code. \$\endgroup\$ – 200_success Jul 8 '15 at 22:31
  • \$\begingroup\$ Yeah sorry i am learning french but i corrected them thank you! \$\endgroup\$ – thomasxd24 Jul 9 '15 at 8:47
2
\$\begingroup\$

I repeated it in at least 4 places

Seeing the same code multiple times should make you want to refactor it into a separate function, like:

void async WaitForReady(Browser browser)
{
    do
    {
        await Task.Delay(500);
    } while (!browser.IsDocumentReady);
}

Notice that this function always waits at least 500ms, which is a long time. Consider changing the loop to a regular while (to allow the possibility of no delay) and reducing the delay amount. Also, this whole approach assumes the best way to find out when the browser is ready is to keep asking it. Does the browser class raise an event when it becomes ready?

You ask whether the while loop "impacts the system a lot". Task.Delay won't really impact at all, it just stops execution along this path. But a user would definitely notice a 500ms delay in an application's UI, so from that perspective it might impact the system. Presumably testing the IsDocumentReady property is efficient, but again maybe there isn't an alternative anyway.

Other than that, your DeleteProduct function does a lot of different things and would be clearer broken into several functions.

\$\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.