[Alyssa Saila]

I have a simple method that reads, processes, and writes back to file.

Using bool return methods, my plan is to stop the remainder of all the
rest of the error checking methods if any unwanted criteria was found.

Reason being of course, if an error surfaced there would not be a need
to continue checking anything else until the error has been corrected.

My goal is to ensure I write this the most simple, yet modularized way
possible.

My question is...

Have I written this the best way to stop the process of error checking
if an error is found in any one of the error checking methods?




Beneath I provided a generic example of the "LoadFile" method.

public void LoadFile(string fPath)
{
	bool noError = false;
	ArrayList myList = new ArrayList();

	if (File.Exists(fPath)) { noError = true; }
	if (noError) { myList = new ArrayList(File.ReadAllLines(fPath)); }
	if (noError) { noError = ErrorCheckForEmptyData(myList); }
	if (noError) { noError = ErrorCheckDelimitedData(myList); }
	if (noError) { noError = ErrorCheckCommentedNotes(myList); }
	if (noError)
	{
		myList = RemoveTrailingBlankLines(myList);
		myList = TrimInteriorBlankLines(myList);
		myList = RemoveDuplicateEntries(myList);
		WriteNewData(myList, fPath);
	}
	else
	{
		MessageBox.Show("an error discovered in file");
	}
}

Beneath I provided a generic example of modularized calls.

// Error Check For Empty Data
private bool ErrorCheckForEmptyData(ArrayList myList) {
	return noError;
}
// Error Check Delimited Data
private bool ErrorCheckDelimitedData(ArrayList myList) {
	return noError;
}
// Error Check Commented Notes
private bool ErrorCheckCommentedNotes(ArrayList myList) {
	return noError;
}
// Remove Trailing Blank Lines
private ArrayList RemoveTrailingBlankLines(ArrayList myList) {
	return myList;
}
// Trim Interior Blank Lines
private ArrayList TrimInteriorBlankLines(ArrayList myList) {
	return myList;
}
// Remove Duplicate Entries
private ArrayList RemoveDuplicateEntries(ArrayList myList) {
	return myList;
}
// Write New Data
private void WriteNewData(ArrayList myList, string fPath) {
	// Writes New Data...
}

[tlhIn`toq]

The normal way to do this is to raise exceptions.

        /// <summary> If all goes right, this method returns a loaded General Preferences object read from disk
        /// 
        /// </summary>
        /// <param name="path"></param>
        /// <returns></returns>
        public Preferences.General LoadFile(string path)
        {
            var temp = new Preferences.General();
            try
            {
                // Load or deserialize a file on disk to the temp obj
            }
            catch (Exception err)
            {

                throw err; // You can re-throw the original exception and deal with it one layer above
                throw new Exception("LoadPreferencesFailed"); // Or throw your own exception
            }
            return temp;
        }

Which you would use like this:

        private Preferences.General myPreferences;
        void StartUpApplication()
        {
            try
            {
               myPreferences = LoadFile(System.IO.Path.Combine(App.UserDirectory, "GeneralSettings.xml"));
            }
            catch (Exception err)
            {
                // Failed to load preferences for whatever reason found in err
                // So we will just make a new default set of preferences
                myPreferences = new General();
            }
        }

[Alyssa Saila]

Back to the drawing board : (

You helped a lot though (and thank you), only problem is I've never even seen this type of code before.

So I'll have to go through my resources and learn about using Preferences.

You pointed me in the right direction and I appreciate that.

[tlhIn`toq]

The preference is just an example. Its a class that I use all the time for my applications, but it is a class that I made. You're not going to find it in .NET.

I don't know what kind of file you are reading or what it's purpose is, but it must being reading into some kind of object for later use. I don't know if that's a SalesReportObject or a PreferenceObj or a BankCustomerObject.

The important thing to take away from this is how the exceptions are thrown and handled. If you want to do that while reading into an array, that's cool.

[Alyssa Saila]

It's the tab delimited file you've helped me on in previous posts.

It has two columns:
leftSide [TAB] rightSide

In that file, I'm error checking for duplicates, extra tabs, and note markers not formatted correctly.

If any one error is found in any one of the error checking methods, all further error checking does not proceed.

If all is well after the error checking, it then loads up the data from it, and eventually writes it back.

All of it working, I just wasn't sure if the way I was checking for errors using the 'If' statements was the best way (which it's not).

From what I take away with what you've provided, I need to place my error checking methods in a try catch block, and catch all of the errors with my own created exception handler (instead of simply stating if (noError) if (noError) if (noError) etc...

This post has been edited by Alyssa Saila: 08 July 2012 - 11:18 AM

[tlhIn`toq]

Personally I would break this into two methods:
One really simple one that just reads a file in.
The other to validate it. The reason being that file formats and contents change over time. Right now your format is just two columns. 6 months from now it might be 3. How do you handle that? How do you handle some customers having file format version 1 and some version 2?

Here's what I would do:

FileContents:
version 1
Bob\t123
Mary\t456
John\t789

Notice the very first item is the version number. Now when you read the file you can send it to the correct validation method

bool LoadFile(string path, ref myList)
{
   try
   {
      myList = new ArrayList(File.ReadAllLines(fPath)); }
   }
   catch
   {
      // Handle error
      return false; // because we failed
   }

   try
   {
      bool success = false;
      switch (myList[0])
      {
          case "version 1":
            success = ValidateVersion1(ref myList);
            break;
          case "version 2":
            success = ValidateVerions2(ref myList);
            break;
       }
       catch
       {
           // handle error
       }
    }
   return success;
}

Now the program can dynamically adjust to newer versions of the datafile. And the methods are void: They return meaningful information about whether or not they failed.

This post has been edited by tlhIn`toq: 08 July 2012 - 11:31 AM

[Alyssa Saila]

Indeed, but there will still be things in both versions that will need the same error checking.

Like checking to see if the file is empty, or commented notes for example.

I implemented code to ignore delimited lines with C style note markers ("//" & "/* */").

That was a headache, but I got it working. However again, because there are multiple methods of error checking (regardless of the number columns (or what type of delimiter I choose), I figured that my options are either to make this one big error checking method, or figure out a way to check the file one module at a time (and break the remainder of the module checking process the instant my criteria for the file is not met).

I could have all of the error checking methods all run even after multiple errors are found (and populate all of the errors at the end), but I figured it would be easier for me to correct them one at time error for error -rather than to try to fix them all at one time.

Although there won't be too many errors, it's still critical of course that the lines are all correct before the file gets written back to.

Also, and I should have mentioned this earlier (my apologies), what I describe as "errors" aren't really system or compiler errors, they're really just lines of data I personally defined as not meeting the criteria needed for the program to run as intended.

This post has been edited by Alyssa Saila: 08 July 2012 - 11:48 AM

[tlhIn`toq]

Alyssa Saila, on 08 July 2012 - 12:48 PM, said:

[size="3"][font="Lucida Sans Unicode"]Indeed, but there will still be things in both versions that will need the same error checking.

Thats fine. Just make those their own methods. You can then call them in whatever order that file version is using

bool validateVersion1()
{
   bool success = CheckCustomerName();
   if (success) success = CheckAccountNumber();
   if (success) success = CheckCustomerAddress();
   return success;
} 

bool validateVersion2()
{
   bool success = CheckCustomerName();
   if (success) success = CheckCustomerAddress();// Notice the different order
   if (success) success = CheckAccountNumber();
   if (success) success = CheckIfCheckingAccount(); //... and the new item to check
   return success;
}

[Alyssa Saila]

Okay I'm following I'm following...

So using the if statements was in fact the best way to achieve this?

if (success) success = CheckAccountNumber();
if (success) success = CheckCustomerAddress();

I know it's one of those questions so fundamentally simple, that with little explanation it can sound complicated (which is my fault really), but I just wanted to make sure I was on the right track with the way I was using all of these if statements.

I read things from time to time about using too many if statements (or how they should be red flags that one is approaching the logic incorrectly). Seeing as I was using the if (noErrors) back to back as I was, - I thought of them as red flags, and figured that perhaps I was doing something wrong.

[tlhIn`toq]

"best" can be a relative term. It starting getting into stylistic choices of how one developer codes versus another. Ask 10 coders what is 'best' and you'll get 10 different answers.

I find this to be a very reasonable way to do what you want to do. You understand it. Its very readable to the next person who takes on your project. And it lends itself to adjustments without having to scrub through hundreds of lines of code.

[Alyssa Saila]

tlhIn`toq you are my coding therapist!

Thanks a lot, you can bill me for two hours! ; )

[Skydiver]

tlhIn'toq makes a good point about style. The current group I work with prefers that if (success) chain style. Another group I worked for preferred that the methods be put in a array and that code would loop through the array and break out when success becomes false. Another group I worked for preferred a compound if statement as it's group style: if(check1() && check2() && check3() ...)

This post has been edited by Skydiver: 08 July 2012 - 05:57 PM

[Alyssa Saila]

Methods in array huh....

Aw man that sounds kinda hot ; )

I might have to look into that.

This post has been edited by Alyssa Saila: 09 July 2012 - 05:35 PM

[Skydiver]

Alyssa Saila, on 09 July 2012 - 05:35 PM, said:

Methods in array huh....

Aw man that sounds kinda hot ; )

I might have to look into that.

I live to turn people on.

Here's a quick and dirty example:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace SimpleCSConsole
{
    class Validator
    {
        List<Func<bool>> _checks = new List<Func<bool>>();

        public void Add(Func<bool> func)
        {
            _checks.Add(func);
        }

        public bool Check()
        {
            foreach (var check in _checks)
            {
                if (!check())
                    return false;
            }
            return true;
        }
    }

    class Foo
    {
        public int Version { get; private set; }
        Func<bool> _versionValidate;

        public Foo(int version)
        {
            switch (version)
            {
                case 1:
                    _versionValidate = ValidateV1;
                    break;

                case 2:
                    _versionValidate = ValidateV2;
                    break;
            }
        }

        public bool Validate()
        {
            return _versionValidate();
        }

        bool ValidateV1()
        {
            Validator validator = new Validator();

            validator.Add(CheckCustomerName);
            validator.Add(CheckAccountNumber);
            validator.Add(CheckCustomerAddress);
            return validator.Check();
        }

        bool ValidateV2()
        {
            Validator validator = new Validator();

            validator.Add(CheckCustomerName);
            validator.Add(CheckCustomerAddress);
            validator.Add(CheckAccountNumber);
            validator.Add(CheckIfCheckingAccount);
            return validator.Check();
        }

        bool CheckCustomerName()
        {
            return true;
        }

        bool CheckAccountNumber()
        {
            return true;
        }

        bool CheckCustomerAddress()
        {
            return true;
        }

        bool CheckIfCheckingAccount()
        {
            return true;
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            var foo1 = new Foo(1);
            foo1.Validate();

            var foo2 = new Foo(2);
            foo2.Validate();
        }
    }
}

[AdamSpeight2008]

You could also utilise the Task<T> primitive and use a Continuation-Passing Style.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;


namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            // Utilising the custom class Valid<T>
            var res = exts.DoesFileExist("s")
                             .LoadFile()
                             .CheckForEmptyData();
            // Utilising the Task<T>
            var res1 = DoesFileExist("s")
                       .ContinueWith<List<String>>(LoadFileIn)
                       .ContinueWith(OutputResult);

        }

        public static Task<string> DoesFileExist(String FilePath)
        {
            return Task.Factory.StartNew<string>(() =>
            {
                var exists = System.IO.File.Exists(FilePath);
                return FilePath;
            });

        }
        private static void OutputResult(Task<List<String>> d)
        {
            if (!d.IsOkToContinue())
            { Console.WriteLine("Errm"); }
            else
            {
                Console.WriteLine(d.Result.Count());
            };

        }

        private static List<String> LoadFileIn(Task<String> filePath)
        {

            if (!(filePath.IsOkToContinue())) return null;
            return new List<string>(System.IO.File.ReadAllLines(filePath.Result));

        }


    }

    public static class exts2
    {
        public static bool IsOkToContinue(this Task t1)
        {
            return (t1 != null) && (t1.IsCompleted) && !(t1.IsCanceled) && !(t1.IsFaulted);
        }
        public static bool IsOkToContinue<T>(this Task<T> t1)
        {
            return t1.IsCompleted && !t1.IsCanceled && !t1.IsFaulted;
        }


    }



    public static class exts
    {

        public static Valid<List<String>> CheckForEmptyData(this Valid<List<String>> Data)
        {
            if (!Data.valid) return new Valid<List<String>>(false, null);
            return new Valid<List<String>>(true, Data.Value);
        }

        public static Valid<List<String>> LoadFile(this Valid<String> ValidPath)
        {
            if (!ValidPath.valid) return new Valid<List<string>>(false, null);
            var lines = new List<String>();
            try
            {
                lines.AddRange(System.IO.File.ReadAllLines(ValidPath.Value));
                return new Valid<List<String>>(true, lines);
            }
            catch (Exception)
            {
                return new Valid<List<String>>(false, null);
            }

        }

        public static Valid<String> DoesFileExist(string fPath)
        {
            try
            {
                return new Valid<String>(System.IO.File.Exists(fPath), fPath);
            }
            catch (Exception)
            {
                return new Valid<String>(false, fPath);
            }

        }

    }



    public class Valid<T>
    {
        public readonly bool valid;
        public readonly T Value;
        public Valid(bool valid, T ReferedTo)
        {
            this.Value = ReferedTo;
            this.valid = valid;
        }
    }
}