For peer review: InputBox.

Paladin2000

Purpose: A simple data entry dialog, functionally similar to MessageBox. Feedback is welcomed.

// Sample Utilization:

//Example 1
string s = string.Empty;
if (InputBox.ShowDialog("Type your name:", ref s) == DialogResult.Cancel) return;

//Example 2
int x = ((int?)InputBox.Create("Multiply by 2:", "1") ?? 0) * 2;

InputBox Class

/// /// Input dialog box used for simple user data entry.
///
public static class InputBox
{
/// /// Standard DialogResult method used for simple user input.
///
/// Title for the Input form.
/// Default to be displayed in the textbox.
/// DialogResult, and updates the value of reference parameter
/// defaultValue if the result is DialogResult.OK.
public static DialogResult ShowDialog(string caption, ref string defaultValue)
{
InputForm inForm = new InputForm(caption, defaultValue);

    if (inForm.ShowDialog() == DialogResult.OK)
    {
        defaultValue = inForm.StringValue;
        return DialogResult.OK;
    }
    return DialogResult.Cancel;
}

/// /// Direct InputBox method, used for immediate typecasting.
/// Shows the dialog as part of its creation.
/// 
/// Title for the Input form.
/// Default to be displayed in the textbox.
/// InputForm ready to be typecast to the appropriate value type.
public static InputForm Create(string caption, string defaultValue)
{
    InputForm inForm = new InputForm(caption, defaultValue);
    if (inForm.ShowDialog() == DialogResult.Cancel) inForm.StringValue = string.Empty;
    return inForm;
}

}

InputForm (used by InputBox)

/// /// Display class for InputBox. Should not be used directly, use InputBox instead.
///
public partial class InputForm : Form
{
#region Constructors

/// /// Default constructor.
/// 
public InputForm()
{
    InitializeComponent();
}

/// /// Parameterized constructor.
/// 
/// Title for the Input form.
/// Default to be displayed in the textbox.
public Inpu

Nagy Vilmos

Can I suggest you post this under the tips-and-tricks. You'll get help with it there and it's more easily found in the future than this will be.

Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

Paladin2000

Agreed, I have copied it there...

Kabwla Phone

I could not find your code under 'tips & tricks' so I will shoot you here... This code has some flaws. You are lying to the user of the code Method "Create" does not only create the form, it also shows it. The showing of the form will halt program execution while a caller of the code might not expect this. You are confusing the user of the code You have a method that is called "Create" it returns a form that has already been shown, and you use the form in a direct comparission with an integer. This will earn you an article on: http://thedailywtf.com/[^]. You are leaky (well the code is) You are creating a form, but never disposing it, this is easily fixed in "ShowDialog" by using a 'using'. But "Create" is returning the form, so who will be responsible for that? Things can get fckd up.

var form1 = InputBox.Create("Multiply by 2:", "1");
// so form one is created, shown, not disposed and still in this reference (with a value of 9)
//get second value.
var form2 = InputBox.Create("Multiply by 2:", "6");
// lets asume that form2 had entered "6"
bool areSame = (int)form1 == (int)form2;
//This is actually true. (on review, have not actually run the code)

Better rethink your strategy to something like:

public static class AskUserForValue
{
public int GetInt() { etc. }
public string GetString() { etc. }
public long GetLong() { etc. }
}

OriginalGriff

Kabwla.Phone wrote:

You are leaky (well the code is)
You are creating a form, but never disposing it, this is easily fixed in "ShowDialog" by using a 'using'.
But "Create" is returning the form, so who will be responsible for that?

Did you miss the static keyword on the class definition?

Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water

Kabwla Phone

The class InputBox is static and there is nothing wrong with that, but it creates multiple instances of non-static (public!!!) class InputForm. These instances need to be disposed otherwise you are leaking memory (and handles). Also in this scenario, I imagine that the point of having a static-class is to hide the implementation of the actual form. In which case the form class should be made private, and the instance should not be returned. If the form is part of a complex mechanism, and you have code that actually needs to be able to reach the form, make it 'internal'. But considering the usage shown, I think this is not the case. You could also consinder making the constructor internal so only the assembly with InputBox can creates an instance of it. Changning the modifiers for the form does not in any way solve any of the other points I mentioned.

BillWoodruff

Just as you possibly did not test the code yourself: "(on review, have not actually run the code);" I also have not really thought through your comments, however, my guess is there's some good points made in them. But, I do think using terms like 'lying to the user' is unnecessary insult to the original poster, who is sharing his work in good faith, here. Bill

Scipio: "That's true, Berganza; and what makes the miracle greater is, that we not only speak, but hold intelligent discourse, as though we had souls capable of reason; whereas we are so far from having it, that the difference between brutes and man consists in this, that man is a rational animal, and the brute is irrational." Cervantes, "Colloquy of Dogs," 1613CE. The two talking dogs, Scipio, Berganza, are hallucinations in the mind of a soldier with plague fever undergoing a "sweating" cure.

Kabwla Phone

If I have come off as offensive, I am sorry. My point, aptly indicated by 'bold statements' were meant to be a lighthearted overexageration of the problem at hand. I merely wanted to provide the requested peer review and give some pointers on how to write quality code. I thank you for your feedback and will consider this when writing another review. If you feel a need to bash on me, or some of my code, then I kindly invide you to visit my programming tip at: Extension method to determine if control is visible in complex hierarchy[^]

Paladin2000

Oh, I wouldn't worry about it, I have pretty thick skin. Besides, the point of the post was review anyway, i.e., "how does this look"? I would not expect only positive answers and agreement; code project is not for my own personal affirmation. ;) I had thought that typecasting the form itself was a novel approach, but perhaps with these valid concerns/objections I will rethink the idea. I shall revisit the concept, and if I resolve them then I will post a revised version. :-D

Paladin2000

As promised, here is the update. (This is the tip/trick location that someone couldn't find, earlier.) InputBox: A simple user input dialog[^]