patterncsharpMinor
Simple C# application that transfers new files (modified in the last 24 hours) from one directory to another
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);
```
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 :)
-
Always check if a file or directory exists before opening:
-
User input should always be compare in one case for example
-
System directories should not be explicitly entered. For example :
-
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
I think that's a good start.
- Your classes and methods should always explicitly define their level of accessebility. e.g.
class Functionsshould bepublic 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.