Read file modify a line and write file back out

Question

2.00/5 (1 vote)

See more:

I have written a function which can read in a file and modify a line and then write it back out. The code is somewhat complicated and convoluted. Please direct me to some sample code where I can accomplish this in a more professional manner.

Thank you.

Here's the code:

C#

public static void manageAllTests(String obr_tests,string obx_tests)
        {
            String[] ind;
            String [] existingElements;
            String[] newElements;
            int found = 0;
            int lineno = -1;
            string elements = "";
            StreamReader textIn = new StreamReader(new FileStream(ReportFolder + "\\allTests.txt", FileMode.OpenOrCreate, FileAccess.Read));
            while (textIn.Peek() != -1)
            {
                lineno += 1;
                string line = textIn.ReadLine();
                ind = line.Split('\t');
                if (ind[0] == obr_tests)
                {
                    found = 1;
                    break;
                }
            }
            textIn.Close();
            if (found == 1)
            {
                int curlineno = 0;
                StringBuilder buffer = new StringBuilder();
                StreamReader textInRem = new StreamReader(new FileStream(ReportFolder + "\\alltests.txt", FileMode.OpenOrCreate, FileAccess.ReadWrite));
                while (textInRem.Peek() != -1)
                {
                    string curline = textInRem.ReadLine();
                    if (curlineno != lineno)
                    {
                        buffer.AppendLine(curline);
                    }
                    else
                    {
                        ind = curline.Split('\t');
                        elements = ind[1];
                    }

                    curlineno += 1;
                }
                textInRem.Close();
                StreamWriter textOutm = new StreamWriter(new FileStream(ReportFolder + "\\alltests.txt", FileMode.Create, FileAccess.Write));
                textOutm.Write(buffer.ToString());
                textOutm.Close();
            }

            existingElements = elements.Split('|');
            newElements = obx_tests.Split('|');
            foreach (string e in newElements)
            {
                int found1 = 0;
                for (int x = 1; x < existingElements.Length; x++)
                {
                    if (e == existingElements[x])
                    {
                        found1 = 1;
                    }

                }
                if (found1 == 0)
                {

                    if ( elements == "")
                    {
                        elements = e;

                    }
                    else
                    {
                            elements = elements + "|" + e;
                    }
                    existingElements = elements.Split('|');
                }
             }

            StreamWriter textOut = new StreamWriter(new FileStream(ReportFolder + "\\alltests.txt", FileMode.Append, FileAccess.Write));
            textOut.WriteLine(obr_tests + '\t' + elements);
            textOut.Close();
            }

Posted 12-Jun-13 11:45am

stevenandler

Add a Solution

Comments

Sergey Alexandrovich Kryukov 12-Jun-13 18:09pm

Any particular problems?
—SA

stevenandler 13-Jun-13 11:22am

Thank you for your honest input. If you can suggest a couple of good C# books for me to read, I would really appreciate it.

Sergey Alexandrovich Kryukov 13-Jun-13 11:45am

I'm not the best adviser here. I did not happen to read anything really good on paper, and did not really need it as I knew many things before those books appeared and later used only original documentation, nearly exclusively. I saw some books bought my other people, and I considered them just unacceptable, but now one can find a lot more. Let's see if some other members suggest you something. And I personally don't appreciate practical books too much, I think it's much more important to learn computer science and then apply it to existing tools.

Anyway, are you accepting my answer formally (green button)?

—SA

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)

Sergey Alexandrovich Kryukov · Answer 1 · 2013-06-12T12:45:00

The list of problems can be much longer than your code iself, and giving you a code sample would hardly teach you much. Instead, you should listen to some criticism, learn what you have to learn, write it again, listen to criticism, and so on. First of all, the code cannot be good or bad by itself (well, it can certainly be bad in principle, of course). It can be good a bad when the goal of it is known, as well as requirements, and this is something you did not bother to specify. So, you cannot hope for a really comprehensive advice, due to the lack of specifications.

Code is showed without the context. Where is "ReportFolder"?

As you use "ReportFolder" in a static method, it means that "ReportFolder" (and maybe something else is also static). This is absolutely bad. Data should not be static. Do you understand threading problems related to static and not existing for stack variables and when you use different instances of complex type in different threads? Anyway, please don't tell us you are not using threads, it won't justify the failure. There is no a reason to declare static data members. At the same time, methods are often required to be static, but then it should be methods using only the parameters and return value or object, nothing else. For instance methods, it's OK and quite appropriate to operate with their declaring type's instance members, which are accessible via "this".

File name should be a parameter of the method, not declared inside it.

Now, the fact that the immediate constant "\\alltests.txt" is declared more then once is not a bug, it's a crime. Do you understand that you just killed the supportability of your project? If you have to have some string constants in your code, it should be explicitly generated as const. Very often, constant strings belong to resource embedded in the executable module or even configuration files.

You should never use repeated concatenations like obr_tests + '\t' + elements). Strings are immutable. Do I even have to explain the implications? You should write instead:

C#

string someContent = string.Format("{0}\t{1}", obr_tests, elements);

Your use of string looks generally very inefficient, by similar reasons.

Not a bug, but: the names like obr_tests violate (pretty good) Microsoft naming conventions. You should not use underscores; and never use non-common abbreviations. Use, say someTests (this is just a hint; instead of "some" should be something semantically sensible).

Never use "", use string.Empty. This is not a bug, but this will greatly help you if you start exterminating immediate constants. Search globally in most files: no "? Great! Many developers never hard-code any immediate constants but 0, 1 and null.

Your pattern of using StreamWriter and other stream-related types is totally wrong. (This will work, but this is actually a bug + plus code design mistake.) Think about having some exception before the stream is closed. Your use of FileStream is totally pointless. Should be:

C#

string someFileName = //...
using (StreamWriter writer = new StreamWriter(someFileName)) {
   writer.Write(buffer.ToString()); 
   // what is "buffer"? If it is a string, .ToString() is redundant
} // writer.Dispose is called automatically here

Please see: http://msdn.microsoft.com/en-us/library/yh598w02.aspx[^].

Your textIn.Peek() looks totally pointless. What did you want to achieve? Maybe you simply need to write all the contents at once, which is the best for more or less small files. You say that you needed to read lines and modify one. Do exactly that, than you won't need anything but ReadLine.

It is most likely that your Split calls require the option StringSplitOptions.RemoveEmptyEntries. Isn't likely that sometimes you may have not tab, but tabs with some accidental space characters? Such inaccurate formatting is hard to detect visually, so use:
http://msdn.microsoft.com/en-us/library/tabh47cf.aspx[^],
http://msdn.microsoft.com/en-us/library/system.stringsplitoptions.aspx[^].

Use FxCop:
http://en.wikipedia.org/wiki/FxCop[^],
http://msdn.microsoft.com/en-us/library/bb429476.aspx[^].

Break your method into smaller methods each having clear goal and simple implementation.

Right, your thinking looks convoluted, but you can train yourself. Write all intermediate steps in plain English. If you can clearly explain what they do, you can easier code them clearly. When you gain good experience, this stage won't be needed, but at first it will teach you a lot.

—SA