Comments
Comments should describe why something is done in the way it is done. Describing what is done does not add any value.
Comments like
// Create form and its fields // Translations // Design form and its fields .....
are also a sign that this parts maybe should be extracted to separate methods.
You can find a very good read about comments here: http://codereview.stackexchange.com/a/90113/29371https://codereview.stackexchange.com/a/90113/29371
Naming
Classes should be named using nouns or noun phrases (naming guideline), so a name like Validate
is better suited for a method. You should rename the class to Validator
or better ProductPriceValidator
to make it more clear what the class is about.
In addition the name Prompt
does not reflect what the class is doing. A better name would be ProductPriceView
.
ShowPriceDialog
This method is in its current state clearly too long. Also, why did you make the class and all of its methods static ?
I would make the class and its methods non static
and would add methods like
InitializeComponent()
for creating the viewResetView()
for setting the default values of the form- and keep the
ShowPriceDialog()
method, but I would change the returned value fromdecimal
toDialogResult
and adding aPrize
property to get the entered prize. This needs a change in the textbox eventhandlers to callHide()
on the view rather thanClose()
.
Instead of using
if (this.textBoxAddProduct.Text.Trim().Length == 0)
you should use string.IsNullOrWhiteSpace()
which is checking wether the passed in string is null
or does only contain white space characters.
Assigning string.Empty
makes the intention more clear than assigning ""
which by the way with older eyes not always is easy to distinguish at the first glance from " "
.
Because you aren't changing these fields
private string defaultTextAddProductTextBox; private string messageMissingProductname;
you should make them readonly
.
Validation
IMHO the IsValidPriceFormat()
has a decent flaw. You are assuming that the entered price will have a comma as decimal separator. If you plan to localize it to make it usable for e.g an american user, you would not only need to change the regex, but to provide a different validator so you won't break the german users.
A much better way to validate the passed price would be to use decimal.TryParse()
which returns a boolean indicating if the given string could be parsed to a deciaml value. At the provided link location you can find a example of how to use this.
A useful component could also be a MaskedTextBox