[slagusev]

How to keep using the String <list> save dialog?

        private void button1_Click(object sender, EventArgs e)
        {
            if (saveConfig.ShowDialog() != DialogResult.OK)
            {
                List<string> configlist = new List<string>();
                configlist.Add('"' + titleTextBox.Text + '"');
                configlist.Add('"' + autorTextBox.Text + '"');
                configlist.Add('"' + saveTextBox.Text + '"');
                configlist.Add('"' + versionTextBox.Text + '"');
                configlist.Add('"' + consoleComboBox.SelectedText + '"');
                configlist.Add('"' + widthTextBox.Text + '"');
                configlist.Add('"' + heightTextBox.Text + '"');
                configlist.Add('"' + fullscreenComboBox.SelectedText + '"');
                configlist.Add('"' + vsyncComboBox.SelectedText + '"');
                configlist.Add('"' + fsaatextBox.Text + '"');
                configlist.Add('"' + joystickComboBox.SelectedText + '"');
                configlist.Add('"' + audiocomboBox.SelectedText + '"');
                configlist.Add('"' + keyboardCombobox.SelectedText + '"');
                configlist.Add('"' + eventCombobox.SelectedText + '"');
                configlist.Add('"' + imagecomboBox.SelectedText + '"');
                configlist.Add('"' + graphicscomboBox.SelectedText + '"');
                configlist.Add('"' + timercomboBox.SelectedText + '"');
                configlist.Add('"' + mousecomboBox.SelectedText + '"');
                configlist.Add('"' + soundcomboBox.SelectedText + '"');
                configlist.Add('"' + physicscomboBox.SelectedText + '"');
                StreamWriter writer = new StreamWriter(saveConfig.FileName);
                foreach (string s in configlist)
                {
                    writer.WriteLine(configlist);
                }
                writer.Close();
            }
            else
            {
                
            }
        }

This code does not create a file when you save in the dialogue(((

[AdamSpeight2008]

What does the logic of the code actually state?

[tlhIn`toq]

First: This is horrible and will break most times.
physicscomboBox.SelectedText
If there is text, but it's not selected then you get null returned.
Learn to use properties and set/get them based on selection changing and text changing.

Writing a text file is always one of the first things people want to do, in order to store data like high-scores, preferences and so on.
Writing a text file tutorial.
Reading a text file tutorial.

[Momerath]

if (saveConfig.ShowDialog() != DialogResult.OK)

This says "if they didn't press the OK button, I want to save the data". You won't have a selected file and the code won't work.

Also, why are you placing " marks around the text? You'll just have to strip them off when you reload the file.

[eclipsed4utoo]

If you simply used a StringBuilder, you wouldn't have to loop through the list at the end to print.

[negligible]

Stringbuilder would also have made adding those double quotes around each string a lot easier to, and easier to change if you need to.

I would use Stringbuilder, in a loop, to Insert & Append a double quote to my String Variables as I added (or removed, either) each item to the list.

So I'd only have to write configlist.Add(item) once.

This post has been edited by negligible: 01 June 2012 - 05:20 AM

[Skydiver]

negligible, on 01 June 2012 - 05:19 AM, said:

Stringbuilder would also have made adding those double quotes around each string a lot easier to, and easier to change if you need to.

I would use Stringbuilder, in a loop, to Insert & Append a double quote to my String Variables as I added (or removed, either) each item to the list.

So I'd only have to write configlist.Add(item) once.

Without using reflection, how would you only write configlist.Add(item) only once? Aren't you just deferring the work?

:
    List<string> configlist = new List<string>();
    foreach(var item in GetItems())
        configlist.Add(item);
:
    StreamWriter writer = new StreamWriter(saveConfig.FileName);
    foreach (string s in configlist)
    {
        writer.WriteLine(configlist);
    }
    writer.Close();


// I'm just guessing at the signature so feel free to change it
IEnumerable<string> GetItems()
{
    // What code goes here?
}

But why even bother with the intermediate StringBuilder? Why not something more direct like:

using(StreamWriter writer = new StreamWriter(saveConfig.FileName))
{
    foreach (var item in GetItems())
        writer.WriteLine("\"{0}\"", item);
}

But then the issue still remains, what code will go into the mythical GetItems() method (without using reflection)? Off the top of my head it will be something like:

IEnumerable<string> GetItems()
{
    yield return titleTextBox.Text;
    yield return autorTextBox.Text;
    yield return saveTextBox.Text;
    yield return versionTextBox.Text;
    yield return consoleComboBox.SelectedText;
    yield return widthTextBox.Text;
    yield return heightTextBox.Text;
    yield return fullscreenComboBox.SelectedText;
    yield return vsyncComboBox.SelectedText;
    yield return fsaatextBox.Text;
    yield return joystickComboBox.SelectedText;
    yield return audiocomboBox.SelectedText;
    yield return keyboardCombobox.SelectedText;
    yield return eventCombobox.SelectedText;
    yield return imagecomboBox.SelectedText;
    yield return graphicscomboBox.SelectedText;
    yield return timercomboBox.SelectedText;
    yield return mousecomboBox.SelectedText;
    yield return soundcomboBox.SelectedText;
    yield return physicscomboBox.SelectedText;
}

Is which case, you may as well have dropped the loop and gone with:

string format = "\"{0}\"";
using(StreamWriter writer = new StreamWriter(saveConfig.FileName))
{
    writer.WriteLine(format, titleTextBox.Text);
    writer.WriteLine(format, autorTextBox.Text);
    writer.WriteLine(format, saveTextBox.Text);
    writer.WriteLine(format, versionTextBox.Text);
    writer.WriteLine(format, consoleComboBox.SelectedText);
    writer.WriteLine(format, widthTextBox.Text);
    writer.WriteLine(format, heightTextBox.Text);
    writer.WriteLine(format, fullscreenComboBox.SelectedText);
    writer.WriteLine(format, vsyncComboBox.SelectedText);
    writer.WriteLine(format, fsaatextBox.Text);
    writer.WriteLine(format, joystickComboBox.SelectedText);
    writer.WriteLine(format, audiocomboBox.SelectedText);
    writer.WriteLine(format, keyboardCombobox.SelectedText);
    writer.WriteLine(format, eventCombobox.SelectedText);
    writer.WriteLine(format, imagecomboBox.SelectedText);
    writer.WriteLine(format, graphicscomboBox.SelectedText);
    writer.WriteLine(format, timercomboBox.SelectedText);
    writer.WriteLine(format, mousecomboBox.SelectedText);
    writer.WriteLine(format, soundcomboBox.SelectedText);
    writer.WriteLine(format, physicscomboBox.SelectedText);
}

This post has been edited by Skydiver: 01 June 2012 - 09:37 AM

[tlhIn`toq]

You really need to embrace the OBJECT part of Object Oriented Programming.

And for the sake of your own sanity and decent coding practice please quit tightly binding to the GUI with all the calls to the GUI elements
physicscomboBox.SelectedText
Use properties, within an object.

A huge problem with what you have designed here is that it is totally bound to the form. You can't deal with the data on its own. If you needed to make a new form in another language, you're screwed. If you wanted to make a new form just for a manager to give them extra options, you're screwed.

This is also a huge maintenance problem. You have hardcoded all the read/write lines to match the GUI. If you add a field then you have to scrub through hundreds of lines of code to add support for that new line. If you forget it someplace then the field isn't written to the file.

What you have here is a bad habit that you want to never get into, because it will be that much harder to break and re-learn a good way of doing this. Anyone that tried to work for our company and showed this as part of their application wouldn't even be interviewed.

Make a class for your ... Whatever this is. When a textbox or combobox makes a change update the correct property of your class instance. Then you can stop this nonsense of micromanaging the text file writing process and just serialize your object.

You want to only every just pass around an instance of your object. Just one thing, not a dozen, not a List<string>

Separating data from GUI - PLUS - serializing the data to XML