4
\$\begingroup\$

Please see the code below:

public bool IsValidateInputSuceeded(string EnteredCost, string EnteredPaymentReceived, out decimal Cost, out decimal PaymentReceived)
{
    bool ValidateInputSuceeded = true;

    EnteredCost= EnteredCost.Substring(EnteredCost.IndexOfAny("0123456789".ToCharArray()));
    EnteredPaymentReceived = EnteredPaymentReceived.Substring(EnteredPaymentReceived.IndexOfAny("0123456789".ToCharArray()));

    if ((Decimal.TryParse(EnteredCost.Replace(" ", "").Replace(",", ""), out Cost)) == false)
    {
        ValidateInputSuceeded = false;
    }

    if ((Decimal.TryParse(EnteredPaymentReceived.Replace(" ", "").Replace(",", ""), out PaymentReceived)) == false)
    {
        ValidateInputSuceeded = false;
    }

    if (ValidateInputSuceeded == false)
    {
        return false;
    }

    if (EnteredCost.IndexOf(" ") > 0|| EnteredPaymentReceived.IndexOf(" ") > 0)
    {
        return false;
    }

    if (EnteredCost[EnteredCost.Length-1] =='.' || EnteredCost[EnteredCost.Length - 2] == '.' || EnteredPaymentReceived[EnteredPaymentReceived.Length - 1]=='.' || EnteredPaymentReceived[EnteredPaymentReceived.Length - 2] == '.')
    {
        return false;
    }

    if (EnteredCost.IndexOf(".") == -1 || EnteredPaymentReceived.IndexOf(".")==-1)
    {
        return false;
    }


    return true;
}

EnteredCost and EnteredPaymentReceived are input parameters from MaskedTextBoxs'(https://msdn.microsoft.com/en-us/library/system.windows.forms.maskedtextbox(v=vs.110).aspx) both with an input mask of: £999,999.00. For example, a value of one of them could be: "£ , 3.22" (without quotes) as an input parameter into the function.

There are two string variables: EnteredCost and PaymentReceived. Should these variables be StringBuilders instead of Strings? The reason I ask is because there are a log of string manipulations going on e.g. using the REPLACE function and the indexOfAny function, which returns more strings.

Improve this question
\$\endgroup\$
10
  • 2
    \$\begingroup\$ Please take a moment to rewrite the title and write what your code is doing and not your concerns about it. If you are not sure how to do it see How do I ask a good question? also please explain the code a little bit more. \$\endgroup\$
    – t3chb0t
    May 7, 2017 at 10:32
  • \$\begingroup\$ StringBuilder helps with concatenation, not replace or index of. Describing the desired validation method might help with suggesting a faster/cleaner approach. \$\endgroup\$
    – D. Jurcau
    May 7, 2017 at 10:38
  • \$\begingroup\$ @D. Jurcau , I have edited the question with more detail. Does that help? \$\endgroup\$
    – w0051977
    May 7, 2017 at 10:47
  • \$\begingroup\$ @t3chb0t, I have edited the question. Does that help? \$\endgroup\$
    – w0051977
    May 7, 2017 at 10:47
  • \$\begingroup\$ Yep, I like it much better now however the title still needs to bo improved. It's validating a payment string so maybe try to write something about it like validating payment string has some format etc. You know better what the method does. The title should reflect this. \$\endgroup\$
    – t3chb0t
    May 7, 2017 at 10:50

1 Answer 1

Reset to default
1
\$\begingroup\$

Since

EnteredCost and EnteredPaymentReceived are input parameters from MaskedTextBox

and a MaskedTextBox

Uses a mask to distinguish between proper and improper user input.

then the validation is not necessary because all it does is to check whether the strings contain numeric data and a dot . as a separator. The MaskedTextBoxs alredy do that so why are you validating their input again?

Improve this answer
\$\endgroup\$
8
  • \$\begingroup\$ The text can appear like this: " , 9.52", which cannot be casted into a decimal or like this: "£ , 9,5 " (space on the end) or like this: "£ , 9. 2" (second to last character is a space). Therefore I believe validation is necessary? \$\endgroup\$
    – w0051977
    May 7, 2017 at 11:58
  • \$\begingroup\$ @w0051977 you could set the TextMaskFormat = MaskFormat.ExcludePromptAndLiterals so the Text property will give you a clean int, you can then easily parse it and multiply with 0.01 and the the actuall value (assuming the mask is something like £ 0.00. \$\endgroup\$
    – t3chb0t
    May 7, 2017 at 12:08
  • \$\begingroup\$ @w0051977 The text can appear like this: " , 9.52", [..] or like this: "£ , 9,5 [..] or like this: "£ , 9. 2" how so? If it's a masked-text-box then why are so many formats allowed? It doesn't make sense. Didn't you set the Mask property? If you didn't then this control hasn't any advantage over an ordinary text-box and this would explain why the manual validation. \$\endgroup\$
    – t3chb0t
    May 7, 2017 at 12:16
  • \$\begingroup\$ the mask is: £999,999.00. This means someone can enter: £ , 9. 2, doesn't it? \$\endgroup\$
    – w0051977
    May 7, 2017 at 12:31
  • 2
    \$\begingroup\$ @w0051977 when I use the Mask = "£999,999.00" it's not possible to enter anything else but a digit or a space so I don't understand where the 50p should come from. The user shouldn't even be able to use the P or any other letter. It's also not possible to type any separators either. \$\endgroup\$
    – t3chb0t
    May 7, 2017 at 13:05

Your Answer

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

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