How to redesign my code?

Question

1.00/5 (1 vote)

See more:

I have an aqward question
How to redesign my code?

I change my mind about the structure of the program.
I want to do what its doing already, but in a different way.
What is doing right now: When [I select a user from list], its going and read the Online page and decrypt it and solving the directives, for every user i click in list.
What I want is a single time button click that will resolve the existing tasks but for ALL users in list. After resolving everything, save ALL (images,bio,etc) to my hard drive, and then load it from there.

So, this is my structure change. I hope I was clear enough.

Before looking at the code, I must specify is not about the code itself. Its working perfectly. Also, I can add by myself anything is needed when the structure of the code is changed.
But its about the order of things. I really hope to be explicit enough.
I know how to reorganize with "cut and paste". Its not such a big deal.
I will in the end, add [everything] in a BIG Method, and the most obvious one will be the button1_Click one.
I am conscientious that will be a MASSIVE method. I dont mind, I did it before and worked perfectly.
What I ask from you, are ideas about reorganizing (more structural perhaps, or logical) than I am already used to do it by myself, which is working great by the way. Maybe you know something that i missed in organizing and planning my structural code thinking, I want something that will actually help me, not confuse me.
Thanks for your understanding.

And here is the code(a part), but is not actually too relevant(I put it anyway).
For the fun of it.

C#

//Decrypt button
private void button1_Click(object sender, EventArgs e)
{
    streamx = ""; file = ""; listBox1.Items.Clear();
    //read ALL "htm" files
    DirectoryInfo di = new DirectoryInfo(Application.StartupPath);
    foreach (var item in di.GetFiles())
    {
        if (item.Extension == ".htm")
        {
            StreamReader sr = new StreamReader(item.FullName);
            file += sr.ReadToEnd();
            sr.Close();
        }
    }
    //extract usernames into listbox
    string cod = "username=\".*?\"";
    Regex rx = new Regex(cod);
    foreach (var item in rx.Matches(file)) listBox1.Items.Add(item.ToString().Replace("username=", "").Replace("\"", ""));
    label1.Text = listBox1.Items.Count.ToString();
    listBox1.SelectedIndex = 0;

    //-----------------------------------
    //Save Files
    //save usernames from your listbox, into the [0.txt]
    StreamWriter sw = new StreamWriter("Artists\\0.txt");
    foreach (var item in listBox1.Items) sw.Write(item + "\r\n");
    sw.Close();


    //Save Images and userBio
    //-----------------------------------
}


List<string> URLs;
string urlx = "", text3564 = "", codRX = "", extrpage = ""; int i7 = 0, i8972 = 0, pagesDeviants = 1;
bool nextpag = false;
public void userBio()
{
    streamx = ""; urlx = "";
    //-http://adamhughes.deviantart.com/
    urlx = "http://" + listBox1.SelectedItem.ToString().ToLower() + ".deviantart.com";
    linkLabel1.Text = urlx;

    //-------------------------------------------------------------
    //Special wepage Reading (extract info from page)
    HttpWebRequest request;
    HttpWebResponse response = null;
    Stream stream = null;
    request = (HttpWebRequest)WebRequest.Create(urlx);
    request.UserAgent = "Foo";
    request.Accept = "*/*";
    response = (HttpWebResponse)request.GetResponse();
    stream = response.GetResponseStream();


    //Request.Cookies  ?
    //Response.Cookies ?



    StreamReader sr = new StreamReader(stream, System.Text.Encoding.Default);
    streamx = sr.ReadToEnd();
    if (stream != null) stream.Close();
    if (response != null) response.Close();
    //-------------------------------------------------------------

    //C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.0\System.Web.dll
    //search information:
    //extract usernames into listbox
    RichTextBox1.Text = listBox1.SelectedItem.ToString() + "  ";
    // <div id="aboutme-personal-info">Poland</div>

    text3564 = streamx;
    if (text3564.Contains("aboutme-personal-info"))
    {
        string cod01 = "aboutme-personal-info\">.*\n.*";
        text3564 = Regex.Match(streamx, cod01).ToString().Replace("aboutme-personal-info\">", "").TrimStart().Replace("</div>", "").TrimEnd();
        RichTextBox1.Text += "[" + text3564 + "]\r\n";
    }

    if (text3564.Contains("aboutme-realname"))
    {
        string cod02 = "aboutme-realname\">.*\n.*";
        text3564 = Regex.Match(streamx, cod02).ToString().Replace("aboutme-realname\">", "").TrimStart().Replace("</div>", "").TrimEnd();
        RichTextBox1.Text += "Real Name= [" + text3564 + "]\r\n";
    }


    //"aboutme-bio";
    text3564 = streamx;
    if (text3564.Contains("START BIO"))
    {
        i7 = text3564.IndexOf("START BIO"); text3564 = text3564.Remove(0, i7);
        if (text3564.Contains("END BIO"))
        {
            i7 = text3564.IndexOf("END BIO"); text3564 = "<" + text3564.Remove(i7, text3564.Length - i7) + ">";
        }

        text3564 = text3564.Replace("<br />", "\r\n").Replace(""", "\"");

        string cod03 = "<.*?>";
        text3564 = Regex.Replace(text3564, cod03, "");
        text3564 = text3564.TrimStart().TrimEnd();


        RichTextBox1.Text += "\r\n Bio= [ " + text3564 + "\r\n]\r\n";
    }

    //avatar Image
    text3564 = streamx;
    if (text3564.Contains("<img class=\"avatar float-left\" src=\""))
    {
        i7 = text3564.IndexOf("<img class=\"avatar float-left\" src=\""); text3564 = text3564.Remove(0, i7);
        string cod05 = "src=\".*?\"";
        text3564 = Regex.Match(text3564, cod05).ToString().Replace("src=", "").Replace("\"", "");
        pictureBox2.ImageLocation = text3564;
    }

    //aboutme-profile-pic
    text3564 = streamx;
    if (text3564.Contains("id=\"aboutme-profile-pic"))
    {
        i7 = text3564.IndexOf("id=\"aboutme-profile-pic"); text3564 = text3564.Remove(0, i7);
        if (text3564.Contains("<div id=\"aboutme-info\">"))
        {
            i7 = text3564.IndexOf("<div id=\"aboutme-info\">"); text3564 = text3564.Remove(i7, text3564.Length - i7);
        }
        string cod04 = "src=\".*?\"";
        text3564 = Regex.Match(text3564, cod04).ToString().Replace("src=", "").Replace("\"", "");
        //textBox1.Text = text3564;
        pictureBox1.ImageLocation = text3564;
    }
    else
    {
        pictureBox1.Image = null;
    }

}
public void userGallery()
{
    pagesDeviants = 1;
    streamx = ""; urlx = "";
    //First Page !!!
    urlx = "http://" + listBox1.SelectedItem.ToString().ToLower() + ".deviantart.com/gallery/";
    linkLabel1.Text = urlx;
    URLs = new List<string>();


start:
    //-------------------------------------------------------------
    //Special wepage Reading (extract info from page)
    HttpWebRequest request;
    HttpWebResponse response = null;
    Stream stream = null;
    request = (HttpWebRequest)WebRequest.Create(urlx);
    request.UserAgent = "Foo";
    request.Accept = "*/*";
    response = (HttpWebResponse)request.GetResponse();
    stream = response.GetResponseStream();

    StreamReader sr = new StreamReader(stream, System.Text.Encoding.Default);
    streamx = sr.ReadToEnd();
    extrpage = streamx;//Next page? >>>



    if (stream != null) stream.Close();
    if (response != null) response.Close();
    //-------------------------------------------------------------



    //-------------------------------------------------------------
    //Extract ALL images from current Page
    //First
    //cut the page at segment: [folderview-art] (to start from correct position)
    text3564 = streamx; codRX = urlx = ""; i7 = 0;
    i7 = text3564.IndexOf("<div class=\"folderview-art\">"); text3564 = text3564.Remove(0, i7);
    //Second
    //Sample Pics x6
    string st0 = "data-super-img=";
    if (text3564.Contains(st0))
    {
        while (text3564.Contains(st0))
        {
            i7 = text3564.IndexOf(st0); text3564 = text3564.Remove(0, i7);
            codRX = st0 + ".*?(\" )";
            urlx = Regex.Match(text3564, codRX).ToString().Replace(st0, "").Replace("\"", "");
            i7 = text3564.IndexOf(urlx); text3564 = text3564.Remove(0, i7 + urlx.Length);
            URLs.Add(urlx);
        }
    }
    //-------------------------------------------------------------


    //-------------------------------------------------------------
    //Next page?
    if (extrpage.Contains("<li class=\"next\">"))
    {
        //no pages left
        if (extrpage.Contains("class=\"next\"><a class=\"disabled\">")) goto end;

        i8972 = extrpage.IndexOf("<li class=\"next\">");
        extrpage = extrpage.Remove(0, i8972);
        //extracting Next page link
        i8972 = extrpage.IndexOf("offset=\"");
        extrpage = extrpage.Remove(0, i8972 + "offset=\"".Length);
        i8972 = extrpage.IndexOf("\"");
        extrpage = extrpage.Remove(i8972, extrpage.Length - i8972);

        urlx = "http://" + listBox1.SelectedItem.ToString().ToLower() + ".deviantart.com/gallery/?offset=" + extrpage;
        pagesDeviants++;
        goto start;
    }
//-------------------------------------------------------------
end: ;
}

private void listBox1_SelectedIndexChanged(object sender, EventArgs e)
{
    label2.Text = "Loading"; listBox1.Update();
    this.Refresh();
    userBio();
    userGallery();
    AutoArrange();
    label2.Text = "Done";
}

Posted 7-May-15 2:13am

_Q12_

Updated 7-May-15 2:24am

v5

Add a Solution

Comments

CHill60 7-May-15 8:25am

First words of advice - don't create a "massive" method - break it down into smaller portions so you can easily follow the flow.
Secondly - don't use the default names for controls ("listBox1", "button1") - give them meaningful names
Thirdly - make your comments meaningful or remove them all together.
Finally - if you want a single button click to handle all of the items in the listbox just step through the list and call the code currently behind the SelectedIndexChanged event for each iteration.

_Q12_ 7-May-15 8:35am

1. It is already break down into small methods - see the listBox1_SelectedIndexChanged is containing the smaller methods inside.
2. not a major problem.
3. Every comment there is specific and I wrote them, personally. I didn't copy them mindlessly. They are very meaningful! Its impossible to make something more "meaningful" than it is already.
4. I just specified that i can make one very BIG ass method that will contain everything, including the step through the list part and iteration. That's my default solving.

CHill60 7-May-15 8:44am

1. Was a response to your words around "MASSIVE"
2. It will become a major problem when you revisit your code
3. "// <div id="aboutme-personal-info">Poland</div>" is not very meaningful - there are other examples.

I've also just noticed the goto start; - use an appropriate loop construction in preference to goto.
As you are using StreamReader you should consider using Using - Understanding the 'using' statement in C#[^]

_Q12_ 7-May-15 9:42am

Can you specify what are other examples? (for#3)
Im not saying its good or bad, im just asking.

_Q12_ 7-May-15 8:37am

Show me some GOOOOOOD sample of GOOOOOOD code.
If you know.

Maciej Los 7-May-15 8:45am

No! It does not work like this!

Maciej Los 7-May-15 8:44am

5ed!

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Sascha Lefèvre · Answer 1 · 2015-05-07T02:48:00

I'll concentrate here on general things, not on what your code actually does, as you're asking for improvements to structurizing code.

- Do not build massive methods. Yes they can work but they're a nightmare to maintain. They have no benefits. Splitting your code into small methods that each serve a single, well defined purpose, makes it much easier to understand, maintain and re-use.

- Do not use labels and goto-statements. It's a shame that these are even part of the C# grammar. Write your code in a way so that it can do without; breaking it up into smaller methods helps a lot there. If you should be interested in becoming a professional programmer: You won't survive the trial period if your boss sees a goto statement in your code. It's the exact opposite of structured programming. http://en.wikipedia.org/wiki/Considered_harmful[^]

- Don't repeat constants as literals (e.g. "START BIO", "END BIO" etc). Define identifiers for them in a central location and use the identifiers wherever needed. If you need to change something, you'll only need to change it in one place.

- Use every object that implements IDisposable in a using-Block (e.g. StreamReader, StreamWriter, HttpWebRequest, HttpWebResponse). Not doing this has the potential to lead to all sorts of strange problems. Using-Blocks also help to visually structure your code. For an explanation, look at one or more of these articles:
https://www.google.com/search?q=idisposable+codeproject&ie=utf-8&oe=utf-8[^]

- If you're serious about structured programming, don't put your business code into the Form (that is, into event-handlers for button-clicks and so on). Instead, choose one of the design patterns for separating the UI from your business code. For Windows Forms you might want to choose MVC or MVP.
https://www.google.com/search?q=windows+forms+mvp+mvc&ie=utf-8&oe=utf-8[^]

edit: And yes, like CHill60 already commented, use meaningful names for your indentifiers (Controls and other variables). If you want to change something about your program after you haven't looked at it for a month or more, you won't remember what textBox5 is supposed to be.