6
\$\begingroup\$

I am going through some CodingAbbey problems to learn basic programming tasks, with a focus on taking input from a file, and saving to another. I have just completed this task:

Of two numbers, please, select one with minimum value. Here are several pairs of numbers for thorough testing.

Input data will contain number of test-cases in the first line. Following lines will contain a pair of numbers to compare each. For Answer please enter the same amount of minimums separated by space, for example:

data:

3
5 3
2 8
100 15

answer:

3 2 15

Here is my code:

using System;

namespace cosnPrjo
{
    class MinimumOfTwo
    {

        const string inputPath = @"/Users/rs/Dropbox/InputFile.txt";
        const string outputPath = @"/Users/rs/Dropbox/OutputFile.txt";
        const string searchAfterParam = "\n";

        static void Main ()
        {
            string input = readFile (inputPath);
            input = input.Substring (input.IndexOf (searchAfterParam) + searchAfterParam.Length);
            string[] pairDelims = { "\n" };
            string[] paramDelims = { " " };
            string[] pairs = input.Split (pairDelims, StringSplitOptions.RemoveEmptyEntries);
            string result = "";

            foreach (string pair in pairs) {
                int numToAppend = 0;
                string[] paramas = pair.Split (paramDelims, StringSplitOptions.RemoveEmptyEntries);

                int firstNum = Convert.ToInt32 (paramas [0]);
                int secondNum = Convert.ToInt32 (paramas [1]);

                numToAppend = firstNum < secondNum ? firstNum : secondNum;  

                result += numToAppend.ToString () + " ";
            }
            writeFile (outputPath, result.TrimEnd ());
        }

        static string readFile (string input)
        {
            return System.IO.File.ReadAllText (input);
        }

        static void writeFile (string path, string contents)
        {
            System.IO.File.WriteAllText (path, contents);
        }
    }
}

My questions are:

  1. Is this method (reading from and writing to files) an effective and/or sensible way to deal with multi-line input? I find it more cumbersome when I am using the console.
  2. Should a StringBuilder be used to append elements to string in this case?
  3. Is the exercise of ignoring the first number, as I have done here, useful or useless in this context? I don't need to use that number, but should I?
  4. This code is pretty long for such a small task. Is it more useful to write shorter code for such tasks?
\$\endgroup\$
4
\$\begingroup\$

There are a few methods that will make the task easier for you:

You might also consider taking the input and output file paths from the args parameter. Putting it all together (without error checking) could look like this:

public static void Main(string[] args)
{
    var minimums = File.ReadLines(args[0]).Skip(1).Select(GetMinimum);
    File.WriteAllText(args[1], string.Join(" ", minimums));
}

private static int GetMinimum(string line)
{
    var numbers = line.Split(' ').Select(int.Parse).ToArray();
    return Math.Min(numbers[0], numbers[1]);
}

I've been lazy here with error checking -- it would be a good exercise to add that to the code. It's also good practice to specify the file encoding when reading and writing files, which again I haven't shown here.

\$\endgroup\$
  • \$\begingroup\$ Can you please explain how to pass in the file through the args parameter? \$\endgroup\$ – alanbuchanan May 11 '15 at 9:02
  • 1
    \$\begingroup\$ @alanbuchanan If running the program from the command line, just type the file paths after the name of the executable: e.g. MinimumOfTwo.exe /Users/rs/Dropbox/InputFile.txt /Users/rs/Dropbox/OutputFile.txt. For Visual Studio, see this answer. For Xamarin, see this answer. \$\endgroup\$ – mjolka May 11 '15 at 9:11
  • \$\begingroup\$ Interesting, thank you. I think this is the first time I've seen string[] args actually made use of. \$\endgroup\$ – alanbuchanan May 11 '15 at 9:15
  • 1
    \$\begingroup\$ in the GetMinimum method, is it not suitable to simply use return numbers.Min(); rather than listing the two array elements? \$\endgroup\$ – alanbuchanan May 11 '15 at 10:45
  • \$\begingroup\$ @alanbuchanan That's a good suggestion! That way you can also remove the call to ToArray. \$\endgroup\$ – mjolka May 11 '15 at 10:47
3
\$\begingroup\$

If you are using a loop to concatenate strings it is always recommended to use a StringBuilder for the task. By using a StringBuilder you won't create a new string for each iteration.


static string readFile (string input)  

You should name the input parameter by its meaning. So better name it fileName.

Based on the naming guidelines methods should be named using PascalCase casing. So readFile() -> ReadFile() and writeFile() -> WriteFile()

static void writeFile (string path, string contents)  

Here it should also be fileName instead of path and content instead of contents.


Instead Convert.ToInt32() I prefer int.Parse() or if I can't trust the given string to be a number I would use int.TryParse()

See also https://stackoverflow.com/q/199470/2655508


You could easily improve your code by replacing the tenary numToAppend = firstNum < secondNum ? firstNum : secondNum; by a simple Math.Min() call.


I would implement a method which is responsible for the parsing of the content of the file. This could return a IEnumerable<T> or a List<T> where I would prefer T to be a Tuple<int,int>.

Another method should then take this collection to evaluate the minimum of each Tuple and returns the result.

This would leave your Main() method to a call to

List<Tuple<int,int>> parseResults = ParseFile(inputPath);

string result = EvaluateMinimums(parseResults);

WriteFile (outputPath, result.TrimEnd());

This would be nice and clean and the implemented methods could be used for other similiar tasks in the future.


Ignoring the first number like you did, is maybe in this szenario ok, but in the long run you will need the number of tests. What I mean is, that for future problems to solve it maybe is needed to know this number, so implementing a method which evaluate the number of tests could be helpful.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.