HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Simple C# application that transfers new files (modified in the last 24 hours) from one directory to another

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lastsimplethenewapplicationdirectoryhoursmodifiedonefiles

Problem

I'm new to C#, so I'm eager for any and all criticism/advice.

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

namespace FileTransfer
{
class Program
{
private static string targetPath;
public static bool running;

static void Main(string[] args)
{
var function = new Functions();
running = true;
int num = 0;

while (running)
{

Console.WriteLine("These files in Customer Orders are new:");
ModifiedFiles newFiles = new ModifiedFiles();
num = 0;

foreach (var file in newFiles.modified())
{
Console.WriteLine("\""+file+"\"");
num++;
}

Console.WriteLine("Transfer {0} file(s) to Home Office? y/n", num);
string answer = Console.ReadLine();

if (answer == "y")
{
targetPath = @"C:\Users\Student\Desktop\Home Office";

num = 0;
foreach (var file in newFiles.modified())
{
File.Copy(file.FullName, Path.Combine(targetPath, file.Name), true);
num++;
}

Console.WriteLine("{0} file(s) transfered.", num);
function.exit();
}

else if (answer == "n")
{
function.exit();
}

else
{
function.wrong();
}
}
}
}
class ModifiedFiles
{
public string sourceDir;
public IEnumerable modified()
{
sourceDir = @"C:\Users\Student\Desktop\Customer Orders";
var directory = new DirectoryInfo(sourceDir);

Solution

A few notes, not in order of importance, just as I see them. Keep in mind this is just my opinions but I think they're important especially since you're starting out. You can form bad habits later :)

  • Your classes and methods should always explicitly define their level of accessebility. e.g. class Functions should be public class Functions



-
Always check if a file or directory exists before opening: var directory = new DirectoryInfo(sourceDir); should be:

DirectoryInfo directory;
if (Directory.Exists(sourceDir))
    directory = new DirectoryInfo(sourceDir);


-
User input should always be compare in one case for example answerExit == "y" should be answerExit.ToLower() == "y" Unless you want to make response case sensitive.

-
System directories should not be explicitly entered. For example : "C:\Users\Student\Desktop\Customer Orders" should be Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.Desktop),"Customer Orders");

-
Your Functions class is superfluous. Those methods directly relate to activities in Main, they can't be used separately or anything else and should definitely be static.

-
Your method modified() return an IEnumerable and I think that this line also return an IEnumerable

var files = directory.GetFiles()
      .Where(file => file.LastWriteTime >= from_date &&         
      file.LastWriteTime <= to_date);` So why return a .ToList()?


I think that's a good start.

Code Snippets

DirectoryInfo directory;
if (Directory.Exists(sourceDir))
    directory = new DirectoryInfo(sourceDir);
var files = directory.GetFiles()
      .Where(file => file.LastWriteTime >= from_date &&         
      file.LastWriteTime <= to_date);` So why return a .ToList()?

Context

StackExchange Code Review Q#131748, answer score: 5

Revisions (0)

No revisions yet.