Не знаю, корректно ли на этом сайте такие вопросы задавать... В общем я не уверен что код ниже верно написан. Свою задачу он выполняет, проверял неоднократно. Однако меня гложут сомнения в том, насколько эта реализация может считаться правильной. Быть может на C# такого рода задачу принято реализовывать иначе? Хотелось бы получить критику и разъяснения.
Сама программа - аудио конвертер. Ищет в папке подходящие файлы и запускает внешний, консольный конвертер для их преобразования в wav.
public partial class Form1 : Form
{
static object locker = new object();
string[] findFiles;
int fifi = -1;
public Form1()
{
InitializeComponent();
Conv();
{
async void Conv()
{
findFiles = Directory.GetFiles(Application.StartupPath, "*.ape", SearchOption.AllDirectories).Union(Directory.GetFiles(Application.StartupPath, "*.ogg", SearchOption.AllDirectories)).ToArray();
// Поддерживаем не более 8 ядер.
int p;
if (Environment.ProcessorCount < 8)
p = Environment.ProcessorCount;
else
p = 8;
int t;
// Если количество файлов больше чем ядер, то устанавливаем столько потоков, сколько ядер.
if(findFiles.Length > p)
t = p;
else // Если файлов меньше чем ядер, то потоков устанавливаем сколько файлов.
t = findFiles.Length;
// Устанавливаем количество потоков.
Task[] tasks = new Task[t];
// Запускаем потоки.
for (int i = 0; i <= p - 1; i++)
{
if(i <= findFiles.Length - 1)
{
tasks[i] = new Task(() => ConvertDoWork(++fifi));
tasks[i].Start();
}
}
// Ждём выполнения всех задач.
await TaskEx.WhenAll(tasks);
Application.Exit();
}
int p, pc;
public void ConvertDoWork(int num)
{
int n = num;
if ((findFiles.Length == 0) || (n > findFiles.Length))
return;
string s = findFiles[n];
ProcessStartInfo startInfo = new ProcessStartInfo();
if (Path.GetExtension(s).Equals(".ogg"))
{
startInfo.FileName = "oggdec.exe";
startInfo.Arguments = "-Q \"" + s + "\"";
}
else
if (Path.GetExtension(s).Equals(".ape"))
{
startInfo.FileName = "MAC.exe";
startInfo.Arguments = "\"" + s + "\" \"" + Path.ChangeExtension(s, ".wav") + "\" -d";
}
startInfo.WorkingDirectory = Application.StartupPath;
startInfo.CreateNoWindow = true;
Process processReg = new Process();
processReg.StartInfo = startInfo;
processReg.StartInfo.WindowStyle = ProcessWindowStyle.Hidden;
processReg.Start();
processReg.WaitForExit();
lock (locker)
File.Delete(s);
int percentComplete = (int)Math.Round((double)(100 * n) / findFiles.Length);
pc = percentComplete / 10;
try
{
this.Invoke((MethodInvoker)delegate
{
progressBar1.Value = percentComplete;
// Если не выбрано не отображать прогресс, то отображаем всплывашку.
if ((notifyIcon1.Visible) && (!chkNotNotify.Checked) && (!pc.Equals(p)))
{
notifyIcon1.ShowBalloonTip(300, "Прогресс", "Выполнено " + percentComplete + "% работы", ToolTipIcon.Info);
p = pc;
}
});
}
catch (System.Exception ex)
{
}
lock (locker)
++fifi;
if (fifi < findFiles.Length)
ConvertDoWork(fifi);
}
Ответ
1.
Слишком много переменных уровня класса. Каждую из них надо отдельно перепроверять - а мне лень. Куда безопаснее локальные переменные и параметры функции.
2.
Переменная fifi используется опасным образом. Лучше от нее вовсе отказаться, а распараллеливание делать другими методами.
Для распараллеливания через задачи я сам использую и всем рекомендую вот такую конструкцию:
await TaskEx.WhenAll(
from file in findFiles
select Task.Run(() => ConvertDoWork(file))
);
Вам, я так понимаю, нужно еще ограничить число число одновременно выполняемых задач числом ядер. Вручную это можно сделать вот так:
class LimitedConcurrencyGuard {
private readonly object _lock = new object();
private readonly Queue
public LimitedConcurrencyGuard (int slots) { this.slots = slots; }
public async Task
Комментариев нет:
Отправить комментарий