5
\$\begingroup\$

From this website I needed to grab the information for each country and insert it into an Excel spreadsheet.

My original plan was to use my program and search each website for the text and later parse that text, but I ran out of time. So what I ended up doing was copying and pasting the information into a text box. With a few characters I was able to separate the countries without too much trouble.

Here is an example of two countries:

>>>Ethiopia

Overview

Some overview information.

Children Available: more information.

Parent Qualifications: more information.

Travel: more information.

Timeline: more information. <<

>>>Ghana

Overview

Some overview information.

Children Available: more information.

Parent Qualifications : more information. //Note the space between Qualifications and the ':'

Travel: more information.

Timeline: more information. <<

This was done to all the countries manually. Add '>>>' > Copy > Paste > add '<<'

Here is my code to parse all this information. This is the form that starts us off:

public partial class Form1 : Form {
    public List<Country> Countries = new List<Country>(); //This never ended up working for me. 

    public Form1() {
        InitializeComponent();
    }

    private void releaseObject(object obj) {
        try {
            System.Runtime.InteropServices.Marshal.ReleaseComObject(obj);
            obj = null;
        } catch (Exception ex) {
            obj = null;
            MessageBox.Show("Exception Occured while releasing object " + ex.ToString());
        } finally {
            GC.Collect();
        }
    }
    /// <summary>
    /// This is where we start everything. We take the text from the textbox and move forward.
    /// </summary>
    private void parse_Click(object sender, EventArgs e) {
        Parser parser = new Parser(textBox1.Text);//We create a new parser object and pass it the text from the textbox.
        parser.parseDocument();

        foreach (var item in parser.CountryTexts) {//This was so I could see we actually had items to deal with.
            Console.WriteLine(item);
        }

        Excel.Application xlApp;//This creates all the thing nessesary to deal with an excel document.
        Excel.Workbook xlWorkBook;
        Excel.Worksheet xlWorkSheet;
        object misValue = System.Reflection.Missing.Value;
        Excel.Range chartRange;

        xlApp = new Excel.Application();
        xlWorkBook = xlApp.Workbooks.Add(misValue);
        xlWorkSheet = (Excel.Worksheet)xlWorkBook.Worksheets.get_Item(1);
        //end setting up excel document.

        //This for loop gets all the text we need. Some of it doesn't come out because of different language used in each specific country.
        for (int i = 0; i < parser.CountryTexts.Count; i++) {
            CountryParser countryParser = new CountryParser();//Creates new CountryParser object.
            countryParser.GetCountry(parser.CountryTexts[i]);//Get all the information from our country. Then put it all in the cells in the WorkSheet using the statements below.
            xlWorkSheet.Cells[i + 1, 1] = countryParser.CountryName;
            xlWorkSheet.Cells[i + 1, 2] = countryParser.Overview;
            xlWorkSheet.Cells[i + 1, 3] = countryParser.ParentQualifications;
            xlWorkSheet.Cells[i + 1, 4] = countryParser.ChildrenProfiles;
            xlWorkSheet.Cells[i + 1, 5] = countryParser.Travel;
            xlWorkSheet.Cells[i + 1, 6] = countryParser.Timeline;
        }

        //Format the cells to my liking.
        chartRange = xlWorkSheet.get_Range("a1", "a100");
        chartRange.WrapText = true;
        chartRange.HorizontalAlignment = 2;
        chartRange.VerticalAlignment = 1;

        chartRange = xlWorkSheet.get_Range("b1", "f100");
        chartRange.EntireColumn.ColumnWidth =27;
        chartRange.WrapText = true;
        chartRange.HorizontalAlignment = 2;
        chartRange.VerticalAlignment = 1;

        //This is where we are saving the file. This was done with an absolute value because I didn't want to deal with making a new file every time I was testing.
        xlWorkBook.SaveAs(@"F:\My Documents\DevWork\OtherProjects\KidBridge\csharp-Excel.xls", Excel.XlFileFormat.xlWorkbookNormal, misValue, misValue, misValue, misValue, Excel.XlSaveAsAccessMode.xlExclusive, misValue, misValue, misValue, misValue, misValue);
        xlWorkBook.Close(true, misValue, misValue);
        xlApp.Quit();

        releaseObject(xlApp);
        releaseObject(xlWorkBook);
        releaseObject(xlWorkSheet);
        this.Close();
        //MessageBox.Show("File created !");
    }
}

This is the parser that separates all the countries:

class Parser {
    public List<Country> Countries = new List<Country>();
    private CountryParser countryParser = new CountryParser();

    string text;
    string[] CountriesText;
    List<int> CountriesStart = new List<int>();
    List<int> CountriesEnd = new List<int>();
    public List<string[]> CountryTexts = new List<string[]>();

    public Parser(string text) {
        this.text = text;
    }

    public void parseDocument() {
        PutTextItArray(text);
        FindCountrySeperators();
        SeperateCountriesInToTextArrays();
    }

    private void PutTextItArray(string text) {//take out text and put it in an array.
        CountriesText = text.Split(' ', '\r', '\n');
    }

    /// <summary>
    /// Here I split up all the countries to make it easier on the CountryParser.
    /// </summary>
    private void SeperateCountriesInToTextArrays() {
        for (int i = 0; i < CountriesStart.Count; i++) {//our loop goes through all the countries using the CountriesStart which we found earlier.
            string[] array = new string[CountriesEnd[i] - CountriesStart[i]];//Find the leangth of the array we need to make.
            Array.Copy(CountriesText, CountriesStart[i], array, 0, CountriesEnd[i] - CountriesStart[i]);//Copy words into that array.
            CountryTexts.Add(array);//Add the array to our CoutryTexts list of string arrays.
        }
    }
    /// <summary>
    /// Here I looked for the brackets that indicate a country's start and end location.
    /// </summary>
    private void FindCountrySeperators() {
        for (int i = 0; i < CountriesText.Length; i++) {
            if (CountriesText[i] == ">>>") {
                CountriesStart.Add(i);
            }
            if (CountriesText[i] == "<<") {
                CountriesEnd.Add(i);
            }
        }
    }
}

This the the CountryParser that takes the seperated countyies and gets the needed information:

class CountryParser {
    Country country = new Country();
    public string Continent, CountryName, Overview, ParentQualifications, ChildrenProfiles, Travel, Timeline;
    string[] countryInformation;

    string overViewStart = "Overview";
    //string ChildrenPrifilesStart = "Children Available:"; //This is something I used when the text was just a string. It's now an array of strings so we can only use one word.
    string ChildrenPrifilesStart = "Available:";
    //string ParentQualificationsStart = "Parent Qualifications:";
    string ParentQualificationsStart = "Qualifications:";
    string TravelStart = "Travel:";
    string timeStart = "Timeline:";

    int overViewStartint = -1;
    int ChildrenPrifilesStartInt = -1;
    int ParentQualificationsStartInt = -1;
    int TravelStartInt = -1;
    int timeStartInt = -1;


    public void GetCountry(string[] countryInformation) {
        this.countryInformation = new string[countryInformation.Length];//Sets the length of the array needed.
        this.countryInformation = countryInformation;//Puts strings in the array.
        FindAllLocations();
        FindAll();
        SetAll();
    }

    //This method finds all the words we need to split up into different sections. 
    //If it doesn't find the word some of the other methods don't work. 
    private void FindAllLocations() {
        overViewStartint = IndexFinder(overViewStart);
        ChildrenPrifilesStartInt = IndexFinder(ChildrenPrifilesStart);
        ParentQualificationsStartInt = IndexFinder(ParentQualificationsStart);
        TravelStartInt = IndexFinder(TravelStart);
        timeStartInt = IndexFinder(timeStart);
    }

    private void FindAll() {
        FindCountry();
        FindOverview();
        FindChildrenProfiles();
        FindParentQualifications();
        FindEstimatedTravelTime();
        FindEstimatedAdoptionProcessTime();
    }

    private void SetAll() {
        country.CountryName = CountryName;
        country.Overview = Overview;
        country.ParentQualifications = ParentQualifications;
        country.ChildrenProfiles = ChildrenProfiles;
        country.Travel = Travel;
        country.Timeline = Timeline;
    }

    private void FindCountry() {
        CountryName = countryInformation[3];//Lame easy way of doing it. At this point it was 2 am before the work was due so I just filled it in by hand if it wasn't found.
    }
    /// <summary>
    /// These next few methods set our country's information in to specific sections. 
    /// If they don't have one of the word's location it will not set anything and that section will need to be done by hand.
    /// </summary>
    private void FindOverview() {
        if (overViewStartint != -1 && ChildrenPrifilesStartInt != -1) {
            Overview = String.Join(" ", countryInformation, overViewStartint + 1, ChildrenPrifilesStartInt - overViewStartint - 3);
        }
    }
    private void FindChildrenProfiles() {
        if (ChildrenPrifilesStartInt != -1 && ParentQualificationsStartInt != -1) {
            ChildrenProfiles = String.Join(" ", countryInformation, ChildrenPrifilesStartInt + 1, ParentQualificationsStartInt - ChildrenPrifilesStartInt - 3);
        }
    }

    private void FindParentQualifications() {
        if (ParentQualificationsStartInt != -1 && TravelStartInt != -1) {
            ParentQualifications = String.Join(" ", countryInformation, ParentQualificationsStartInt + 1, TravelStartInt - ParentQualificationsStartInt - 3);
        }
    }

    private void FindEstimatedTravelTime() {
        if (TravelStartInt != -1 && timeStartInt != -1) {
            if (TravelStartInt > timeStartInt) {
                Travel = String.Join(" ", countryInformation, TravelStartInt + 1, countryInformation.Length - TravelStartInt - 1);

            } else { 
            Travel = String.Join(" ", countryInformation, TravelStartInt + 1, timeStartInt - TravelStartInt - 3);
        }}
    }

    private void FindEstimatedAdoptionProcessTime() {
        if (timeStartInt != -1) {
            Timeline = String.Join(" ", countryInformation, timeStartInt, countryInformation.Length - timeStartInt);
        }
    }

    //This finds where a given word is in the array.
    private int IndexFinder(string findThisSrting) {
        int integer = -1;
        for (int i = 0; i < countryInformation.Length; i++) {
            if (countryInformation[i] == findThisSrting) {
                integer = i;
            }
        }
        return integer;
    }

Here is the Country class:

public class Country {
    public string Continent, CountryName, Overview, ParentQualifications, ChildrenProfiles, Travel, Timeline;
}

I hadn't added it because it just help strings and they were public.

Here is the Form class:

 partial class Form1 {
    /// <summary>
    /// Required designer variable.
    /// </summary>
    private System.ComponentModel.IContainer components = null;

    /// <summary>
    /// Clean up any resources being used.
    /// </summary>
    /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
    protected override void Dispose(bool disposing) {
        if (disposing && (components != null)) {
            components.Dispose();
        }
        base.Dispose(disposing);
    }

    #region Windows Form Designer generated code

    /// <summary>
    /// Required method for Designer support - do not modify
    /// the contents of this method with the code editor.
    /// </summary>
    private void InitializeComponent() {
        System.ComponentModel.ComponentResourceManager resources = new System.ComponentModel.ComponentResourceManager(typeof(Form1));
        this.Parse = new System.Windows.Forms.Button();
        this.textBox1 = new System.Windows.Forms.TextBox();
        this.SuspendLayout();
        // 
        // Parse
        // 
        this.Parse.Location = new System.Drawing.Point(12, 0);
        this.Parse.Name = "Parse";
        this.Parse.Size = new System.Drawing.Size(75, 23);
        this.Parse.TabIndex = 2;
        this.Parse.Text = "Parse";
        this.Parse.UseVisualStyleBackColor = true;
        this.Parse.Click += new System.EventHandler(this.parse_Click);
        // 
        // textBox1
        // 
        this.textBox1.Location = new System.Drawing.Point(12, 29);
        this.textBox1.MaxLength = 3276700;
        this.textBox1.Multiline = true;
        this.textBox1.Name = "textBox1";
        this.textBox1.Size = new System.Drawing.Size(1242, 613);
        this.textBox1.TabIndex = 3;
        this.textBox1.Text = resources.GetString("textBox1.Text");
        // 
        // Form1
        // 
        this.AutoScaleDimensions = new System.Drawing.SizeF(6F, 13F);
        this.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font;
        this.ClientSize = new System.Drawing.Size(1266, 654);
        this.Controls.Add(this.textBox1);
        this.Controls.Add(this.Parse);
        this.Name = "Form1";
        this.Text = "Form1";
        this.ResumeLayout(false);
        this.PerformLayout();

    }

    #endregion

    private System.Windows.Forms.Button Parse;
    private System.Windows.Forms.TextBox textBox1;
}

This is what starts the program:

 static class Program {
    /// <summary>
    /// The main entry point for the application.
    /// </summary>
    [STAThread]
    static void Main() {
        Application.EnableVisualStyles();
        Application.SetCompatibleTextRenderingDefault(false);
        Application.Run(new Form1());
    }
}

My questions to you are these:

  • Is there a better way to do this? As you can see if the words are not exact for the IndexFinder() then the CountryParser class will not find the needed information.

  • How would I be able to get that information?

  • Would I need to find every case and then use the IndexFinder() with every case?

  • Lastly, am I getting OOP right?

\$\endgroup\$
6
  • \$\begingroup\$ xPath is a really nice tool to have for something like this \$\endgroup\$
    – Malachi
    Oct 27, 2014 at 20:37
  • 1
    \$\begingroup\$ where is the country class? what does it do? \$\endgroup\$
    – Malachi
    Oct 27, 2014 at 20:56
  • 1
    \$\begingroup\$ I don't think that this code is functional, you are missing a class or something and half of the stuff in the code needs to be commented because you are missing the Country class. and then there is the comment that you couldn't get it to work with the List of Countries. also you are missing the Form Load method so the form won't load. \$\endgroup\$
    – Malachi
    Oct 27, 2014 at 21:37
  • \$\begingroup\$ I don't know about the form not loading. It loads for me. I added the County class and the Form class which should at least help in loading the program into Visual Studio. I didn't realize that's what you guys would try to do. I just thought you guys would just look at code and offer pointers on what to change. Thanks for your help. \$\endgroup\$
    – Funlamb
    Oct 28, 2014 at 0:59
  • \$\begingroup\$ you are missing the Form1_Load Method, it takes in arguments and sets up the form, if you don't have it, it won't load. I will look at it again later if I have some time today, and see if I can get it to work. \$\endgroup\$
    – Malachi
    Oct 28, 2014 at 15:52

1 Answer 1

7
\$\begingroup\$

Form1

Please, please, please rename this class. There is nothing more uterlessly insignificant than a class named Class1, an event named Event, a method named Method1, a button named Button1, ...or a form named Form1. Use meaningful names.

public List<Country> Countries = new List<Country>(); //This never ended up working for me. 

Good to hear. But this line is dead code, and the comment is pure noise; code in a code base shouldn't be its own history - there's source control for that. Delete dead code, without mercy.

private void releaseObject(object obj) {

This is by no means obligatory, but conventions place the scope-opening curly brace on the next line. I like that you are consistent about your brace style though, so kudos for that!

Naming conventions also state that methods should be PascalCase - and the code you're showing us is not consistent about that - sometimes it's camelCase, sometimes it's PascalCase - I double-checked the tags to make sure I wasn't answering a question!

Reading the code file top to bottom is a little off-putting, since this releaseObject function just pops out of nowhere and so one comes across the function and mentally scans the next couple of lines underneath to try and find a usage. I'd put private methods like this below where they're used, for a better reading flow.

/// <summary>
/// This is where we start everything. We take the text from the textbox and move forward.
/// </summary>
private void parse_Click(object sender, EventArgs e) {

XML comments are awesome... on public members (to document the API, as XML comments are picked up by IntelliSense). In private methods, they're more or less useless. And this particular comment isn't very useful at all, as it states what the code does - good comments should say why, not what. I'd remove it altogether.

parse_Click sounds like you've got a Button named parse - "parse" is a verb, which makes it a good name for a method... but not for an object. I'd rename it ParseButton, and rename the handler OnParseButtonClick - give that Ctrl+R, R shortcut some lovin'!

Again the comments are just noise here, they say nothing that the code doesn't already say.

Event handlers shouldn't implement logic; there shouldn't be more than a handful of lines of code in there, delegating the work to more specialized methods.

Parser parser = new Parser(textBox1.Text);//We create a new parser object and pass it the text from the textbox.
parser.parseDocument();

Well that's surprising. I'd have expected parseDocument() to return something, and the line to read something like this:

var result = parser.Parse();

Actually... why do we even need a Parser instance? And what is it parsing anyway? "Parser" is as vague as it gets. Perhaps CountryParser could be clearer, with a static IEnumerable<Country> Parse(string text) signature... but from scrolling further down your code I see that there's also a CountryParser class! That's a little bit confusing.

That said, having both a Parser and a CountryParser class is also confusing because at a glance, there seems to be an inheritance relationship between the two: it sounds as if CountryParser would be derived from Parser... yet it's not the case.

I'll get back to the parsers. Now back to the crowded event handler.

foreach (var item in parser.CountryTexts) {//This was so I could see we actually had items to deal with.
    Console.WriteLine(item);
}

Don't do that. Instead, place a breakpoint, and *inspect parser.CountryTexts while debugging, by hovering the identifier. Visual Studio has pretty good debugger tools, use them - don't mix test/debug code with production code!

Actually, that seems like a pretty good reason to write a number of unit tests for your Parser class, so that you can pass it various inputs and see what tests pass and what tests fail, and why.

The method then creates an Excel instance, creates a new workbook (I don't think you need to specify a missing value here), gets the first worksheet, and then proceeds to populate it with the list of countries.

  • Getting an Excel instance is one thing.
  • Creating a new workbook is another.
  • Populating a worksheet with some values, is yet another thing.
  • Formatting the worksheet to your liking is another thing.
  • [Getting a filename, ] Saving & closing a workbook is something else.
  • Quitting & releasing the Excel instance is another thing.

Divide and conquer - move that code elsewhere, it doesn't belong in a button click event handler! Each bullet point above should be the body of a distinct method.

Once you've got a bunch of small methods that do very few things, look at what you've got, and consider moving them to specialized classes - the Single Responsibility Principle applies at that level, too. Good luck!


Country

The Country class, as written, is a BIG HUGE NO-NO!

public string Continent, CountryName, Overview, ParentQualifications, ChildrenProfiles, Travel, Timeline;

These are public fields, which shouldn't be exposed at all. You should consider an immutable struct to hold this data:

public struct Country
{
    public Country(string continent, string name, ...)
    {
        _continent = continent;
        _name = name;
    }

    private readonly string _continent;
    public string Continent { get { return _continent; } }

    private readonly string _name;
    public string Name { get { return _name; } }

    // ...
}
\$\endgroup\$
1
  • \$\begingroup\$ Wow. That's a lot of information. Thanks and I'll be looking into all this. \$\endgroup\$
    – Funlamb
    Oct 28, 2014 at 1:03

Your Answer

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

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