[.net] Comments on PropReader

Started by
6 comments, last by Black Knight 17 years ago
Im pretty new to C# so show some mercy :] I tried to create a propreader or whatever you call it.It is supposed to read key and values from a txt file. For example the init.txt file has :

//these are comments
//another comment
    //comment hehe :)

WindowWidth 100
WindowHeight 200
WindowPosX 300
WindowPosY  = 300

    Fullscreen 1

this wont be read
this
is
going
to
be
ignored
//comment too :)


The reader will only read key value pairs.It basically works now i think :) It also handles the key = value or key : value situations. I just want some advice if there is anything I should correct or improve in it. Thanks in advance. And the class :

namespace O3LEngine
{
	struct tPropVar
	{
		public string szName;
		public string szValue;
	}
	
	//
	//This class will read text files for config files
	//	
	public class PropReader
	{
		private ArrayList m_PropList;


		public bool readFile(string szFile)
		{
			FileStream file;
			tPropVar itemToAdd;
			char[] seperators = new char[] { '=', ':', ' ', '\t' };
			try
			{
				file = File.OpenRead(szFile);

				m_PropList = new ArrayList();

				StreamReader reader = new StreamReader(file);
				string szLine;
				
				while((szLine = reader.ReadLine())!=null){
					string[] strings = szLine.Split(seperators, StringSplitOptions.RemoveEmptyEntries);
					
					//If the entry is empty or one word or it has more than 2 ignore it
					if(strings.Length < 2 || strings.Length > 2)
						continue;
					//if it starts with // then it is a comment
					if(strings[0].StartsWith("//"))
						continue;
					
					itemToAdd = new tPropVar();
					itemToAdd.szName = strings[0];
					itemToAdd.szValue = strings[1];

					m_PropList.Add(itemToAdd);
					
				}
				
				reader.Close();
				file.Close();
			}
			catch(IOException)
			{
				MessageBox.Show("Can't find file " + szFile,"Error!",MessageBoxButtons.OK);
				return false;
			}
			return true;
		}
	}
}


Advertisement
I haven't tried to run your class, so my comments are merely style related. [smile]

First and foremost is the naming convention; I tend to write code that follows the .NET conventions - at least for public members. In this case, then, that'd be:

public bool ReadFile(string filename)

I'd also make public members properties (even if they don't do anything exceptional), so my struct would be:

struct PropVar{	private string name;	public string Name	{		get { return this.name; }		set { this.name = value; }	}	public string value;	public string Value	{		get { return this.value; }		set { this.value = value; }	}	public override string ToString()	{		return string.Format("{0}={1}", this.Name, this.Value);	}}


I would also nest PropVar inside the class itself, as it's pretty meaningless outside the context of the class.

I like being able to just Console.WriteLine() a variable, hence the ToString() method.

Really, though, I'd prefer to use .NET's built-in types for this; KeyValuePair<T, T> is provided for us already. In the same vein I'd probably prefer to return a Dictionary<T, T> of key/value pairs. In this specific instance there are specialised classes - such as the StringDictionary - that would be more appropriate.

Furthermore, I'd rather the class automatically read the file if I passed it the filename (or a stream) in the constructor.

The exception handling is rather strange; if something goes wrong throw the exception and let the application using your class handle it. Do not use return codes to check the status.

You could, I suppose, write an extra method that doesn't throw any exceptions (or display any message boxes) with the name TryReadFile if there is a strong chance of dodgy input.

if(strings.Length < 2 || strings.Length > 2);// Could be (slightly more intuitively):if(strings.Length != 2);


Take a look at the using statement for your file streams and readers. If you must wrap an exception handler around your code, don't forget a finally block to clean up your open files!

I hope I don't come across as rude, that is not my intention.

[Website] [+++ Divide By Cucumber Error. Please Reinstall Universe And Reboot +++]

if you're using C# and .NET, then you have access to the settings management system for typesafe settings like this. In visual studio, right click your project and select Properties. On the Properties page, select the Settings tab. Here, you can give names to different settings, specifying their serialization type, whether it's an application or user level setting, and a default value for that setting. Then in your code you would reference the values as:
global::{your base namespace}.Properties.Settings.Default.{setting name}

obviously, replacing {your base namespace} and {setting name} with the values appropriate for your project.

[Formerly "capn_midnight". See some of my projects. Find me on twitter tumblr G+ Github.]

You could also look at using XML as well.
benryves thanks for the usefull information.
I'll look into those classes already provided.
About the exception handling.Shall I only put the try block inside the function and put the catch block in the application that uses my class?BTW this class is part of a class library.And if I understand correctly if im doing exception handling then i dont need to return an error code.

Some thing like this in my app :

try{readfile();}catch{//handle exception}finally{//close files}


So if the file is not found it will throw an exception or should I explicitly throw an exception if the file reference is null.


You can try this code, it is easier to read in files with this code


using System;using System.IO;using System.Text.RegularExpressions;namespace Utilities{    /// <summary>    /// Parse settings from ini-like files    /// </summary>    public class IniFile    {        static IniFile()        {            _iniKeyValuePatternRegex = new Regex(                @"((\s)*(?<Key>([^\=^\s^\n]+))[\s^\n]*                # key part (surrounding whitespace stripped)                \=                (\s)*(?<Value>([^\n^\s]+(\n){0,1})))                # value part (surrounding whitespace stripped)                ",                RegexOptions.IgnorePatternWhitespace |                RegexOptions.Compiled |                RegexOptions.CultureInvariant);        }        static private Regex _iniKeyValuePatternRegex;        public IniFile(string iniFileName)        {            _iniFileName = iniFileName;        }        public string ParseFileReadValue(string key)        {            using (StreamReader reader =                  new StreamReader(_iniFileName))            {                do                {                    string line = reader.ReadLine();                    Match match =                        _iniKeyValuePatternRegex.Match(line);                    if (match.Success)                    {                        string currentKey =                                match.Groups["Key"].Value as string;                        if (currentKey != null &&                       currentKey.Trim().CompareTo(key) == 0)                        {                            string value =                              match.Groups["Value"].Value as string;                            return value;                        }                    }                }                while (reader.Peek() != -1);            }            return null;        }        public string IniFileName        {            get { return _iniFileName; }        }    private string _iniFileName;    }}
Why does regex syntax look like my grandmoms soup?
I thought c# was designed to make more readable code but regex always confuses me.

This is what I have now.
But im not sure on how to get the values from the class.
At the moment it returns the value in the string passed as a parameter.
In this case I need to convert them to int float or whatever in the calling application.Should I add functions to the class like getFloatValue getIntValue and do the conversions in the class or let the application do those.

Another thing i would like to ask is do I have to close both the file and the streamreader?
I have seen some examples that only close the streamreader.
Here is the class.
using System;using System.Collections.Generic;using System.IO;namespace O3LEngine{	/// <summary>	/// This class reads in txt files with key value pairs	/// </summary>	public class PropReader	{		Dictionary<string,string> m_List;		public PropReader(){}		public PropReader(string file)		{			FileStream readFile;			readFile = File.OpenRead(file);						char[] seperators = new char[] { '=', ':', ' ', '\t' };			m_List = new Dictionary<string,string>();			StreamReader reader = new StreamReader(readFile);			string szLine;			while ((szLine = reader.ReadLine()) != null) {				string[] strings = szLine.Split(seperators, StringSplitOptions.RemoveEmptyEntries);				//If the entry is empty or one word or it has more than 4 ignore it				if (strings.Length != 2)					continue;				//if it starts with // then it is a comment				if (strings[0].StartsWith("//"))					continue;				m_List.Add(strings[0],strings[1]);			}			reader.Close();			readFile.Close();		}		public void getValue(string key,ref string value)		{			m_List.TryGetValue(key,out value);		}	}}

This topic is closed to new replies.

Advertisement