Click here to Skip to main content
15,885,216 members
Please Sign up or sign in to vote.
2.50/5 (2 votes)
See more:
Hello, i'm just wondering if this is a bad way of coding. When i'm coding C#, i'm creating new objects in every method I make kinda.. so I want to know if there is a better way of writing code or am I supposed to do like this? StreamReader etc, i'm creating StreamReader sr = new StreamReader alot and WebClient.

Also, if you find something else that I could improve/think about when coding just let me know.

Example of one of my codes:
C#
int version()
{
    string versionFile = "version.xpv";
    int version;

    if (!File.Exists(versionFile))
    {
        using (StreamWriter sw = new StreamWriter(versionFile))
        {
            sw.WriteLine("0");
        }
    }

    using (StreamReader sr = new StreamReader(path))
    {
        version = Convert.ToInt32(sr.ReadLine());
        sr.Dispose();
    }

    return version;
}

int checkUpdates()
{
    int lines = 0;
    WebClient wc = new WebClient();
    using (StreamReader sr = new StreamReader(wc.OpenRead("http://127.0.0.1/patches/version.txt")))
    {
        while (sr.ReadLine() != null)
        {
            lines++;
        }
    }
    return lines;
}

string GetPatch(int version)
{
    WebClient wc = new WebClient();

    using (StreamReader sr = new StreamReader(wc.OpenRead("http://127.0.0.1/patches/version.txt")))
    {
        for (int i = 0; i < version; i++)
        {
            sr.ReadLine();
        }

        return sr.ReadLine().Substring(2);
    }
}


//Thanks
Posted
Updated 18-May-13 14:48pm
v2
Comments
AlphaDeltaTheta 18-May-13 21:41pm    
For WebClient, this is the recommended approach,
WebClient doesn't support concurrent operations, i.e, if one DownloadString is active, another OpenRead cannot be called on the same object. If at all, one calls occurs while the previous is not complete, the code will fail.
if you're sure that no concurrent calls will occur then you may cache that instance. But be sure to create the instance only when needed. Also be warned that, if it's a private field, it may also be garbage collected after a while.
Paulo Zemek 18-May-13 21:41pm    
Usually it is better to cache a value instead of reading the file every time. But to cache the value, it is important to know if such value is acessed from a single thread or from many threads.
Also, I see a sr.Dispose() when you are already using a using clause, so you don't need to dispose (as the using clause creates a dispose at the end).
Sergey Alexandrovich Kryukov 18-May-13 21:45pm    
What is "object of object"? :-)
—SA

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



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900