First post, long time reader.
I'm currently learning C# with "Head First C#" (I'm up to Encapsulation and Get/Set properties)
I'm writing a small program to work with pictures for a friend, I'm just wondering if I'm heading along the right lines with my PictureController class? My main problem is that I am setting a lot of form items with this class, and it feels unnatured to keep referencing form items from within the class, I'm pasting my code below, if you could let me know if I'm doing anything wrong then I'd be most grateful :)
Many thanks!
PictureController.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;
using System.Windows.Forms;
namespace PictureController
{
class PictureController
{
private int arrayPosition = 0;
private int numFiles = 0;
private string[,] arrayPictures;
public PictureBox myPictureBox;
public RadioButton myCopyButton;
public RadioButton myDeleteButton;
public TextBox mySource;
public ComboBox myDestinations;
private FolderBrowserDialog sourceFolder;
private FolderBrowserDialog destFolder;
public void InitialisePicture()
{
if (arrayPictures != null && arrayPictures.Length > 0)
{
myPictureBox.ImageLocation = arrayPictures[arrayPosition, 0];
}
else
{
MessageBox.Show("The folder you have selected contains no pictures...");
myPictureBox.ImageLocation = null;
}
}
public void NavigatePicture(int direction)
{
if (arrayPosition + direction >= 0 && arrayPosition + direction < numFiles)
{
arrayPosition += direction;
myPictureBox.ImageLocation = arrayPictures[arrayPosition, 0];
myCopyButton.Checked = Convert.ToBoolean(arrayPictures[arrayPosition, 1]);
myDeleteButton.Checked = Convert.ToBoolean(arrayPictures[arrayPosition, 2]);
}
}
public void UpdateActions(bool copyChecked, bool deleteChecked)
{
if (arrayPictures != null)
{
arrayPictures[arrayPosition, 1] = copyChecked.ToString();
arrayPictures[arrayPosition, 2] = deleteChecked.ToString();
}
}
public void GetFiles()
{
sourceFolder = new FolderBrowserDialog();
sourceFolder.ShowDialog();
if (sourceFolder.SelectedPath != "")
{
string[] arrayTempFiles = Directory.GetFiles(sourceFolder.SelectedPath,"*.jpg");
numFiles = arrayTempFiles.Length;
arrayPictures = new string[arrayTempFiles.Length,3];
for (int i = 0; i < arrayTempFiles.Length; i++)
{
arrayPictures[i, 0] = arrayTempFiles[i];
arrayPictures[i, 1] = "false";
arrayPictures[i, 2] = "false";
}
mySource.Text = sourceFolder.SelectedPath;
InitialisePicture();
}
}
public void AddDestinationFolder()
{
destFolder = new FolderBrowserDialog();
destFolder.ShowDialog();
if (destFolder.SelectedPath != "")
{
myDestinations.Items.Add(destFolder.SelectedPath);
}
}
}
}
Form1.cs
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
namespace PictureController
{
public partial class Form1 : Form
{
PictureController PicControl;
public Form1()
{
InitializeComponent();
PicControl = new Pictu开发者_C百科reController() { myPictureBox = pbPhoto, myCopyButton = rbMove, myDeleteButton = rbDelete, mySource = tbSource, myDestinations = cbDestination };
}
private void btnPrev_Click(object sender, EventArgs e)
{
PicControl.NavigatePicture(-1);
}
private void btnNext_Click(object sender, EventArgs e)
{
PicControl.NavigatePicture(1);
}
private void rbMove_CheckedChanged(object sender, EventArgs e)
{
if (rbMove.Checked || rbDelete.Checked)
{
PicControl.UpdateActions(rbMove.Checked, rbDelete.Checked);
}
}
private void rbDelete_CheckedChanged(object sender, EventArgs e)
{
if (rbMove.Checked || rbDelete.Checked)
{
PicControl.UpdateActions(rbMove.Checked, rbDelete.Checked);
}
}
private void Form1_KeyDown(object sender, KeyEventArgs e)
{
switch (e.KeyCode)
{
case Keys.A:
PicControl.NavigatePicture(-1);
break;
case Keys.D:
PicControl.NavigatePicture(1);
break;
case Keys.W:
rbMove.Checked = true;
break;
case Keys.S:
rbDelete.Checked = true;
break;
}
}
private void btnGetFiles_Click(object sender, EventArgs e)
{
PicControl.GetFiles();
}
private void btnProcess_Click(object sender, EventArgs e)
{
}
private void btnAddDest_Click(object sender, EventArgs e)
{
PicControl.AddDestinationFolder();
}
}
}
I don't see the reason to use Controls in your PictureController
class. You should only use non-forms datatypes there and handle the interaction in your Form
, offering events and methods from your PictureController
class to react and act on it.
It's a good start in my opinion.
Hard to tell if you're doing something "wrong" because it depends what you think is right, every programmer has his/her own style and best practice set and as long as the code is working and efficient it's "right". There are many roads leading to Rome. :)
Anyway if you ask for personal opinion or advice, I would make two major changes in logic:
- Have the Controller be a static class (Singleton if you prefer)
- Don't pass or use the form controls directly. Instead pass the instance of your form to the static class in some Initialize method, then use that instance and call public method that is working with the controls directly.
Example for the second change:
public static void NavigatePicture(int direction)
{
if (arrayPosition + direction >= 0 && arrayPosition + direction < numFiles)
{
arrayPosition += direction;
_formInstance.SetPictureLocation(arrayPictures[arrayPosition, 0]);
_formInstance.SetCopyStatus(Convert.ToBoolean(arrayPictures[arrayPosition, 1]));
_formInstance.SetDeleteStatus(Convert.ToBoolean(arrayPictures[arrayPosition, 2]));
}
}
//...and in the form:
public SetPictureLocation(sLocation)
{
myPictureBox.ImageLocation = sLocation;
}
What is the point of having the controller, just encapsulate the extra logic out of the form? Do you need your controller to be testable? Do you want to use an abstract model of your business logic? If you answer yes to last 2 questions you might want to google:
a. MVC pattern b. MVP pattern
Everything is wrong.
Controller (if it is a controller of the form/window) should know about form while in your sample - form know about controller.
Controller should know how to build data / and handle some form events when in your sample controller have references to pictureBox'es etc.
And there are no need to "extract" controller from the control code, while doesn't established what the "data/model" of the control is (and not framed with the type).
It's not clear what is the data there: path or image collection.
精彩评论