2
\$\begingroup\$

I wrote this code (except the getting text from clipboard part) a while back, and I'm wondering if it could be written better? Can anyone help me improve this code?

This code basically takes HTML from the clipboard, and then attempts to fix the indentation and writes it to a new file.

public static class Clipboard
{
    [DllImport("user32.dll")]
    static extern IntPtr GetClipboardData(uint uFormat);
    [DllImport("user32.dll")]
    static extern bool IsClipboardFormatAvailable(uint format);
    [DllImport("user32.dll", SetLastError = true)]
    static extern bool OpenClipboard(IntPtr hWndNewOwner);
    [DllImport("user32.dll", SetLastError = true)]
    static extern bool CloseClipboard();
    [DllImport("kernel32.dll")]
    static extern IntPtr GlobalLock(IntPtr hMem);
    [DllImport("kernel32.dll")]
    static extern bool GlobalUnlock(IntPtr hMem);

    const uint CF_UNICODETEXT = 13;

    public static string GetText()
    {
        if (!IsClipboardFormatAvailable(CF_UNICODETEXT))
        return null;
        if (!OpenClipboard(IntPtr.Zero))
        return null;

        string data = null;
        var hGlobal = GetClipboardData(CF_UNICODETEXT);
        if (hGlobal != IntPtr.Zero)
        {
            var lpwcstr = GlobalLock(hGlobal);
            if (lpwcstr != IntPtr.Zero)
            {
                data = Marshal.PtrToStringUni(lpwcstr);
                GlobalUnlock(lpwcstr);
            }
        }
        CloseClipboard();

        return data;
    }
}

class Program
{
    static string fix_encoding(string src_str)
    {
        src_str = src_str.Replace("a€‹", "");
        src_str = src_str.Replace("&", "&");
        src_str = src_str.Replace(" ", " ");
        src_str = src_str.Replace("‎", "");
        src_str = src_str.Replace("&", "&");
        src_str = src_str.Replace(" ", " ");
        src_str = src_str.Replace("'", "'");
        src_str = src_str.Replace(""", "\"");
        return src_str;
    }

    static string slice(string src_str, int start, int end)
    {
        if (start < 0)
        {
            start = src_str.Length + start;
        }
        if (end < 0)
        {
            end = src_str.Length + end;
        }
        return src_str.Substring(start, end-start);
    }

    static Boolean prefix_check(string item, string[] tag_exception)
    {
        string b = item.ToLower();
        for (var i=0; i<tag_exception.Length; i++)
        {
            if (b.IndexOf(tag_exception[i]) == 0)
            {
                return true;
            }
        }
        return false;
    }

    static void fix_tags(string src_str, StreamWriter file_id)
    {
        src_str = fix_encoding(src_str);

        int count = 0;
        List<String> item_list = new List<String>();

        while (src_str != "")
        {
            int start_tag_index = src_str.IndexOf("<");
            if (start_tag_index == -1)
            {
                break;
            }

            item_list.Add(slice(src_str, 0, start_tag_index));
            src_str = slice(src_str, start_tag_index, src_str.Length);
            count++;

            int _g_ = src_str.IndexOf("<!--");
            if (_g_ == 0)
            {
                int _f_ = src_str.IndexOf("-->");
                item_list.Add(slice(src_str, _g_, _f_ + 3));
                count++;
                src_str = slice(src_str, _f_ + 3, src_str.Length);
                continue;
            }

            int end_tag_index = src_str.IndexOf(">");    
            string tag = slice(src_str, 0, end_tag_index + 1);
            item_list.Add(tag);
            count++;
            src_str = slice(src_str, end_tag_index + 1, src_str.Length);
        }

        string[] tag_exception = {"<img","<link","<meta","<br","<hr","<img","<input","<area","<param"};
        string indent = "";
        Boolean script = false;

        for (var i = 0; i < count; i++)
        {
            string item = item_list[i];
            if (item.Trim() == "")
            {
                continue;
            }
            int start = item.IndexOf("<");
            int end = item.IndexOf(">");
            int end2 = item.IndexOf("</");

            Boolean is_script = item.IndexOf("<script")==0;
            Boolean is_end_script = item.Length >= 9 && slice(item, -9, item.Length) == "</script>";
            Boolean is_tag = start == 0 && end == item.Length-1;
            Boolean is_closing_tag = end2 == 0;
            Boolean is_comment = item.IndexOf("<!") == 0;
            Boolean is_exception = prefix_check(item, tag_exception);

            if (script)
            {
                if (is_end_script)
                {
                    indent = slice(indent,0,-1);   
                    file_id.Write(indent + item.Trim().Replace("\n","") + "\n"); 
                    script = false;
                }
                else {
                    file_id.Write(indent + item.Trim() + "\n");
                }
            }
            else if (is_comment)
            {
                file_id.Write(indent + item.Trim() + "\n");
            }
            else if (is_closing_tag)
            {
                indent = slice(indent, 0, -1);
                file_id.Write(indent + item.Trim().Replace("\n", "") + "\n");
            }
            else if (is_exception)
            {
                file_id.Write(indent + item.Trim().Replace("\n", "") + "\n");
            }
            else if (is_tag)
            {
                file_id.Write(indent + item.Trim().Replace("\n", "") + "\n");
                indent += "\t";  
                if (is_script)
                {
                    script = true;
                }
            }
            else {
                file_id.Write(indent + item.Trim() + "\n");
            }
        }
    }

    static void Main(string[] args)
    {
        StreamWriter file_id = new StreamWriter("./fixed_html.html", false, System.Text.Encoding.UTF8);
        fix_tags(Clipboard.GetText(), file_id);
        file_id.Close();
        System.Diagnostics.Process.Start("batch.bat");
    }
}
\$\endgroup\$
2
  • 3
    \$\begingroup\$ Quick superficial suggestions. I believe CamelCasing is the standard convention for c# variables and Pascalcase method calls. StyleCop would go along way to addressing this. Also fix_tags seems kinda large. Perhaps breaking it into smaller routines might be a start? See also Microsoft naming guidelines - msdn.microsoft.com/en-us/library/xzf533w0%28VS.71%29.aspx \$\endgroup\$
    – dreza
    Commented Jul 11, 2012 at 21:04
  • 1
    \$\begingroup\$ Quick comment: get rid of your own Clipboard class and start using the one that is provided by the framework. msdn.microsoft.com/en-us/library/kz40084e.aspx#Y0 \$\endgroup\$
    – Leonid
    Commented Aug 15, 2012 at 16:20

1 Answer 1

2
\$\begingroup\$
  • What everyone else said about camelCase & style-cop
  • slice: need to be careful if start and end values are valid.
  • prefix_check: why not put incoming items into hash to this goes from order(n) operation to order(1)?
  • fix_tags: name is unclear. Suggest: Adjust tabbing or somesuch.
  • fix_tags: maybe a better way to do this entire method is to load the document into a .NET DOM and then print out that DOM with your preferred indentation.
  • fix_tags: g, f etc need readable names
  • fix_tags: calls to slice are magic-number city, save calculation ints to named values or use string.Length
  • fix_tags: tag_exception: exception to what, need comment
  • fix_tags: tag_exception: as mentioned above make into dictionary or lookup etc.
  • fix_tags: as mentioned by other reviewers this method is too large, suggested refactor locations: the while, the for and then further within those blocks.
  • fix_tags: suggest change bool calculations and repetitive code with a switch containing a few cases and default of: file_id.Write(indent + item.Trim().Replace("\n", "") + "\n");
\$\endgroup\$

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.