C #: Мини Прикладное Структурное Проектирование (Классы / Интерфейсы / и т. Д.) - PullRequest
4 голосов
/ 14 февраля 2010

Я создаю небольшое приложение, которое позволяет пользователю конвертировать изображения в различные размеры и форматы. Я изо всех сил пытался получить хороший твердый дизайн с этим приложением. У меня есть приложение и работает, но оно интегрирует хороший объектно-ориентированный дизайн. Поскольку это личный проект, я хотел узнать больше об интеграции интерфейсов, хорошем наследовании классов, составлении объектов и других элементах ОО-дизайна.

Однако я изо всех сил пытался это сделать. Не поймите меня неправильно, я знаю о дизайне ОО и что это такое, я просто не знаю, как реализовать хороший дизайн ОО в проектах. Конечно, легко взглянуть на примеры занятий, которые вы читаете в книгах или онлайн. Примеры могут иметь простые сценарии, такие как следующие.

Интерфейс IPerson имеет функции-члены Walk () , Run () . Абстрактный класс Person использует Интерфейс IPerson . Класс Man и Класс Female наследуется от Abstract Class Person .

но когда дело доходит до Реальные проекты Я изо всех сил пытаюсь реализовать хороший дизайн. Я надеялся на некоторое понимание. Вот что у меня сейчас есть.

Интерфейс:

interface IPicture
{
    Bitmap ReturnImage(string path, int width, int height);
}

Основной класс, который содержит информацию об изображении. Этот класс в основном хранит информацию о переданном изображении и информацию о новых значениях, которые хочет пользователь (то есть новый размер, новое местоположение файла, новый формат изображения и т. Д.)

public class MyPictures : IPicture
{
    //All Private variables below are properties.  Property get/set's have been removed
    //for the sake of space
    private int _NewWidth;
    private int _NewHeight;
    private string _NewImgName;
    private string _NewImgPath;
    private string _NewImgFullPath;
    private ImageFormat _NewImgFormat;
    //Declare variables to hold values that have been determined
    private int _OldWidth;
    private int _OldHeight;
    private string _OldImgName;
    private string _OldImgPath;
    //Old Image Format is in String format because of certain extension scenarios.
    private string _OldImgFormat;

         public MyPictures(Image img, string file)
    {
        ClearProperties();
        //...set properties based on passed variables in constructor...
    }
    public void ClearProperties()
    {
        _NewWidth = 0;
        _NewHeight = 0;
        _NewImgName = "";
        _NewImgPath = "";
        _NewImgFullPath = "";
        _NewImgFormat = null;
        _OldWidth = 0;
        _OldHeight = 0;
        _OldImgName = "";
        _OldImgPath = "";
        _OldImgFormat = null;
    }

    public override string ToString()
    {  
        return _OldImgPath;
    }
    public void ImageSave()
    {
        Bitmap tempBmp = new Bitmap(_OldImgPath);
        Bitmap bmp = new Bitmap(tempBmp, _NewWidth, _NewHeight);
        bmp.Save(_NewImgPath + @"\" + _NewImgName + "." +  _NewImgFormat.ToString().ToLower(), _NewImgFormat);
    }
    public Bitmap ImageClone()
    {
        Bitmap bmp = new Bitmap(_OldImgPath);
        return bmp;
    }
    Bitmap IPicture.ReturnImage(string path, int width, int height)
    {
        return new Bitmap(new Bitmap(path), width, height);
    }
}

основной класс; Начальная точка приложения. Это определенно нуждается в некоторой работе ...

public partial class Form1 : Form
{
    static bool hasThreadBeenStopped = false;
    static bool imageProcessingComplete = false;
    static bool imgConstrained = false;
    //Default text when user selects 'All' checkbox for new image name
    static string newNameDefault = "'Name' + #";
    Utility.Validation.Validate valid = new Utility.Validation.Validate();

    public Form1()
    {
        InitializeComponent();
        //Populate Combo Box With Possible Image Formats...
        //Conditionally show Image Properties...
        ImgPropertiesEnabled();
        //Set static progress bar properties...
        progressBar1.Minimum = 0;
        progressBar1.Step = 1;
    }

    private void Form1_Load(object sender, EventArgs e)
    {
        lblImgProcessed.Text = "";
        lblFile.Text = "";
        txtContentFolder.Text = "";
    }
    //Delegate declarations.  Used for multi-thread processing
    public delegate void PopulateTextboxDelegate(Label lbl, string text);
    public delegate void ThreadWorkDelegate(Label lbl, string text);
    public delegate void ImageDisplayDelegate(Image i);
    public delegate void ProgressBarDelegate(ProgressBar p, int step, int value);

    //Populate textbox fields with image processed, and image path being processed
    public void PopulateTextbox(Label lbl, string text)
    {
        lbl.Text = "";
        lbl.Text = text;
    }
    public void ThreadWork(Label lbl, string text)
    {
        this.Invoke(new PopulateTextboxDelegate(PopulateTextbox),
                    new object[] { lbl, text });
    }
    //Display Currently Processed Image
    public void ImageDisplay(Image i)
    {
        pbMain.Image = null;
        pbMain.Image = i;
    }
    public void ThreadWorkImg(Image i)
    {
        this.Invoke(new ImageDisplayDelegate(ImageDisplay),
                    new object[] {i});
    }
    //Increment Progress Bar
    public void ProgressBarDisplay(ProgressBar pg, int max, int value)
    {
        //Dynamically set the Progress Bar properties
        pg.Maximum = max;
        pg.Value = value;
    }
    public void ThreadProgress(ProgressBar p, int max, int value)
    {
        this.Invoke(new ProgressBarDelegate(ProgressBarDisplay),
                    new object[] { p, max, value });
    }        
    private void btnStart_Click(object sender, EventArgs e)
    {
        string IsValidResult = IsValid();
        //If string is empty, Utility passed
        if (IsValidResult == "")
        {
            Thread t = new Thread(new ThreadStart(ProcessFiles));
            t.Start();
        }
        else
        {
            MessageBox.Show(IsValidResult);
        }
    }
    public void ProcessFiles()
    {
        int count = 0;

        ThreadWorkDelegate w = ThreadWork;
        ImageDisplayDelegate im = ThreadWorkImg;
        ProgressBarDelegate pb = ThreadProgress;

        try
        {
            foreach (MyPictures mp in lstHold.Items)
            {
                try
                {
                    if (hasThreadBeenStopped == false)
                    {
                        //Disable certain controls during process.  We will use the generic
                        //MethodInvoker, which Represents a delegate that can execute any method 
                        //in managed code that is declared void and takes no parameters.
                        //Using the MethodInvoker is good when simple delegates are needed.  Ironically,
                        //this way of multi-thread delegation was used because the traditional way as used
                        //by the rest of the delegates in this method, was not working.
                        btnApply.Invoke(new MethodInvoker(delegate { btnApply.Enabled = false; }));
                        btnStart.Invoke(new MethodInvoker(delegate { btnStart.Enabled = false; }));

                        //Call delegate to show current picture being processed
                        im.BeginInvoke(mp.ImageClone(), null, null);
                        mp.ImageSave();

                        //Increment Count; Image has been processed
                        count++;

                        //Invoke Img Proceessed Output
                        w.BeginInvoke(lblImgProcessed, count.ToString() +
                                      " of " + lstHold.Items.Count.ToString() + " processed",
                                      null, null);
                        //Invoke File Process Output
                        w.BeginInvoke(lblFile, mp.NewImgPath, null, null);

                        //Invoke Progressbar output.  Delegate is passed The count of images,
                        //which will be set as the progressbar max value.  the 'count' variable is
                        //passed to determine the current value.
                        pb.BeginInvoke(progressBar1, lstHold.Items.Count, count, null, null);
                    }
                    else //Thread has been called to stop
                    {
                        MessageBox.Show("Image Processing Stopped: " + count + "of " +
                                        lstHold.Items.Count + " processed");
                        //Enable controls after process
                        btnApply.Invoke(new MethodInvoker(delegate { btnApply.Enabled = true; }));
                        btnStart.Invoke(new MethodInvoker(delegate { btnStart.Enabled = true; }));
                        break;
                    }
                }
                catch (Exception ex)
                {
                    MessageBox.Show("Error while processing pictures");
                    break;
                }
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show("Error while attempting to execute pictures: " + ex.ToString());
        }
        finally
        {
            //Loop has ended:
            //In finally statement, re-enable disabled controls
            //Enable certain controls during process
            btnApply.Invoke(new MethodInvoker(delegate { btnApply.Enabled = true; }));
            btnStart.Invoke(new MethodInvoker(delegate { btnStart.Enabled = true; }));
            //Reset class variables
            hasThreadBeenStopped = false;
            imageProcessingComplete = false;
        }
    }

    private void btnContent_Click(object sender, EventArgs e)
    {
        string selection = null;
        string[] files = null;

        lstAll.Items.Clear();
        contentBrowser.ShowDialog();

        selection = contentBrowser.SelectedPath;
        txtContentFolder.Text = selection;
        if (selection != "" || selection != null)
        {
            try
            {
                files = System.IO.Directory.GetFiles(selection.Trim());
                foreach (string file in files)
                {
                    lstAll.Items.Add(file);
                }
            }
            catch (Exception ex)
            {
               // MessageBox.Show(ex.ToString());
            }

        }
    }

    private void btnGo_Click(object sender, EventArgs e)
    {
        //Grab files from folder based on user input in the textbox.  
        string selection = txtContentFolder.Text.Trim();
        string[] files = null;

        lstAll.Items.Clear();

        if (valid.IsNull(selection) == false || valid.IsEmpty(selection) == false)
        {
            try
            {
                files = System.IO.Directory.GetFiles(selection);
                foreach (string file in files)
                {
                    lstAll.Items.Add(file);
                }
            }
            catch (Exception ex)
            {
               MessageBox.Show("Invalid Directory");
            }
        }
        txtContentFolder.Text = selection;
    }
    private void btnDestination_Click(object sender, EventArgs e)
    {
        string selection = null;
        destinationBrowser.ShowDialog();
        selection = destinationBrowser.SelectedPath;
        txtNewImgPath.Text = selection;
    }

    private void exitToolStripMenuItem_Click(object sender, EventArgs e)
    {
        this.Close();
    }

    private void btnStop_Click(object sender, EventArgs e)
    {
        //Flag variable that the stop button has been called.  This variable is checked
        //conditionally when looping over each picture.
        hasThreadBeenStopped = true;
    }

    public string IsValid()
    { 
        StringBuilder sb = new StringBuilder("");

        if (lstHold.Items.Count <= 0)
        {
            return "No items exist to process";
        }
        //Validate that there is a value in each field for every object in lstHold.  All the fields will be
        //validated.  Note:  If there is one invalid field, the rest do not need to be considered.  
        foreach (MyPictures mp in lstHold.Items)
        {
            if (mp.NewImgName == "")
            {
                sb.Append(mp.OldImgPath + ", ");
            }
            else if (mp.NewImgPath == "")
            {
                sb.Append(mp.OldImgPath + ", ");
            }
            else if (mp.NewImgFormat == null)
            {
                sb.Append(mp.OldImgPath + ", ");
            }
            else if (mp.NewWidth == 0)
            {
                sb.Append(mp.OldImgPath + ", ");
            }
            else if (mp.NewHeight == 0)
            {
                sb.Append(mp.OldImgPath + ", ");
            }
        }
        //If the returned string is empty, the image is valid.  The check for the listbox's count
        //will return a string immediatly if false.  Because of this, we know that the returning
        //string at this level will either be empty (validation passed) or filled with image paths
        //of images missing required values.  If image is not valid, return this concatenated string of image paths
        //that are missing values, and insert a prefixed string literal to this list.
        if (sb.ToString() != "")
        {
            sb.Insert(0, "The following images are missing required values: ");
            return sb.ToString();
        }
        else //String is empty and has passed validation
        {
            return sb.ToString();
        }

    }

    private void btnMoveOne_Click(object sender, EventArgs e)
    {
        //Loop through All strings in the lstAll list box.  Then use each picture path to convert 
        //each picture into their own class
        foreach (string file in lstAll.SelectedItems)
        {
            //isImgExistFlag is a flag indicating wheter the image coming from lstAll already exists
            //in lstHold.  By default, the variable is false.  It is set to true if an image does exist
            //This variable must be re-created within the scope of the main foreach loop to ensure a proper
            //reset of the variable for each image comparison.
            bool isImgExistFlag = false;
            try
            {
                Image img;
                img = Image.FromFile(file);

                MyPictures mp = new MyPictures(img,file);

                //If lstHold contains no items, add the item with no validation check.  
                if (lstHold.Items.Count == 0)
                {
                    lstHold.Items.Add(mp);
                }
                else
                {
                    //Run through each object in the lstHold to determine if the newly created object
                    //already exists in list box lstHold.
                    for (int i = 0; i < lstHold.Items.Count; i++)
                    {
                        MyPictures p = (MyPictures)lstHold.Items[i];
                        //Unique objects will be identified by their Original Image Path, because
                        //this value will be unique
                        if (p.OldImgPath == mp.OldImgPath)
                        {
                            isImgExistFlag = true;
                        }
                    }
                    //If isImgExistFlag is false, the current Image object doesnt currently exist 
                    //in list box.  Therefore, add it to the list.  
                    if (isImgExistFlag == false)
                    {
                        lstHold.Items.Add(mp);
                    }
                }

            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.ToString());
            }                    
        }
      }

    private void btnMoveAll_Click(object sender, EventArgs e)
    {
        //This event has the same functionality as btnMoveOne_Click, except the main foreach loop
        //is based on all of lstAll's items, rather than just the selected items.
        foreach (string file in lstAll.Items)
        {
            bool isImgExistFlag = false;
            try
            {
                Image img;
                img = Image.FromFile(file);

                MyPictures mp = new MyPictures(img, file);

                if (lstHold.Items.Count == 0)
                {
                    lstHold.Items.Add(mp);
                }
                else
                {
                    for (int i = 0; i < lstHold.Items.Count; i++)
                    {
                        MyPictures p = (MyPictures)lstHold.Items[i];

                        if (p.OldImgPath == mp.OldImgPath)
                        {
                            isImgExistFlag = true;
                        }
                    }
                    if (isImgExistFlag == false)
                    {
                        lstHold.Items.Add(mp);
                    }
                }

            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.ToString());
            }
        }
    }

    private void btnRemoveOne_Click(object sender, EventArgs e)
    {
        /*
        Create a seperate List to populate:
        This is necessary because if you explicitly remove an item from the listbox
        you will get the following error:
        "List that this enumerator is bound to has been modified. An enumerator can 
        only be used if the list does not change."
         */
        //This variable will keep track of the first index processed. 
        int first_index = 0;
        int count = 0;
        List<MyPictures> TempMp = new List<MyPictures>();

        if (lstHold.Items.Count >= 1)
        {
            try
            {
                foreach (MyPictures mp in lstHold.SelectedItems)
                {
                    if (count == 0)
                    {
                        first_index = lstHold.SelectedIndex;
                    }
                    //Add objects to be removed
                    TempMp.Add(mp);
                }
                foreach (MyPictures mp2 in TempMp)
                {
                    lstHold.Items.Remove(mp2);
                }
            }
            catch (Exception ex)
            {
                //Hide Error: MessageBox.Show(ex.ToString());
            }
            //Select new item in list if possible, as long as there is a item in the list
            if (lstHold.Items.Count >= 1)
            {
                //If the first_index variable = the amount of items in the list, the new selected index
                //should be the first index -1.  This is because the variable first_index would be the 
                //index of the now deleted item in the list.  Therefore we must subtract the variable by 1 
                //before assigning it to the selected value.  Otherwise, we'll be assigning a selected index that
                //no longer exists. 
                //There is also a check to make sure there is more than one item in the list.  Otherwise, we could
                //potentially assign a selected index of -1.
                if (first_index == lstHold.Items.Count && lstHold.Items.Count != 1)
                {
                    lstHold.SelectedIndex = first_index - 1;
                }
                else if (lstHold.Items.Count == 1)
                {
                    lstHold.SelectedIndex = 0;
                }
                else
                {
                    lstHold.SelectedIndex = first_index;
                }
            }
            else
            {
                ClearTextBoxes();
            }
        }

    }

    private void btnRemoveAll_Click(object sender, EventArgs e)
    {
        lstHold.Items.Clear();
        ClearTextBoxes();
        ImgPropertiesEnabled();
    }

    private void lstHold_SelectedIndexChanged(object sender, EventArgs e)
    {
        //This prevents trying to access a negative index.  This can happen when a item is removed.
        if (lstHold.SelectedIndex >= 0)
        {
            try
            {
                MyPictures mp = (MyPictures)lstHold.Items[lstHold.SelectedIndex];
                txtOldName.Text = mp.OldImgName;
                txtOldImgPath.Text = mp.OldImgPath;
                txtOldImgFormat.Text = mp.OldImgFormat.ToString();
                txtOldWidth.Text = mp.OldWidth.ToString();
                txtOldHeight.Text = mp.OldHeight.ToString();

                txtNewName.Text = mp.NewImgName;
                cbFormat.SelectedItem = mp.NewImgFormat;
                txtNewWidth.Text = mp.NewWidth.ToString();
                txtNewHeight.Text = mp.NewHeight.ToString();
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.ToString());
            }
        }
        //Call function to determine which controls should be enabled/disabled
        ImgPropertiesEnabled();
    }

    private void btnApply_Click(object sender, EventArgs e)
    {

        //Reset color.  It could be grey depending on if user changed default name.
        txtNewName.ForeColor = Color.Black;

        if (lstHold.SelectedIndex == -1)
        {
            MessageBox.Show("Picture not selected.  Select picture to apply properties to.");
        }
        else if (lstHold.SelectedIndex >= 0)
        {
            MyPictures mp = (MyPictures)lstHold.Items[lstHold.SelectedIndex];

            //User wants to apply a generated name to all pictures within the list
            if (chkNewPicName.Checked == true)
            {
                int count = 0;
                foreach (MyPictures pic in lstHold.Items)
                {
                    pic.NewImgName = txtNewName.Text + count.ToString();
                    ++count;
                }
                txtNewName.Text = mp.NewImgName;
            }
            //User wants to apply a custom name to this picture only
            else
            {
                mp.NewImgName = txtNewName.Text;
            }
            //User wants to apply this path to all pictures within the list
            if (chkNewPicPath.Checked == true)
            {
                foreach (MyPictures pic in lstHold.Items)
                {
                    pic.NewImgPath = txtNewImgPath.Text;
                }
                txtNewImgPath.Text = mp.NewImgPath;
            }
            //User wants to apply this path to this picture only
            else
            {
                mp.NewImgPath = txtNewImgPath.Text;
            }
            //User wants to apply this image format to all pictures within the list
            if (chkNewPicFormat.Checked == true)
            {
                foreach (MyPictures pic in lstHold.Items)
                {
                    pic.NewImgFormat = (ImageFormat)cbFormat.SelectedItem;
                }
            }
            //User wants to apply this image format to this picture only
            else
            {
                mp.NewImgFormat = (ImageFormat)cbFormat.SelectedItem;
            }
            //User wants to apply this size to all pictures 
            if (chkNewSize.Checked == true)
            {
                foreach (MyPictures pic in lstHold.Items)
                {
                    pic.NewWidth = Convert.ToInt32(txtNewWidth.Text);
                    pic.NewHeight = Convert.ToInt32(txtNewHeight.Text);
                }
                txtNewWidth.Text = mp.NewWidth.ToString();
                txtNewHeight.Text = mp.NewHeight.ToString();
            }
            //User wants to apply this size to this picture only
            else
            {
                mp.NewWidth = Convert.ToInt32(txtNewWidth.Text);
                mp.NewHeight = Convert.ToInt32(txtNewHeight.Text);
            }

            mp.NewImgName = txtNewName.Text;
            mp.NewImgFormat = (ImageFormat)cbFormat.SelectedItem;
            mp.NewWidth = Convert.ToInt32(txtNewWidth.Text);
            mp.NewHeight = Convert.ToInt32(txtNewHeight.Text);
        }
    }

    private void checkBox1_CheckedChanged(object sender, EventArgs e)
    {
        if (chkSelectAll.Checked)
        {
            chkNewPicName.Checked = true;
            chkNewPicPath.Checked = true;
            chkNewPicFormat.Checked = true;
            chkNewSize.Checked = true;
        }
        else
        {
            chkNewPicName.Checked = false;
            chkNewPicPath.Checked = false;
            chkNewPicFormat.Checked = false;
            chkNewSize.Checked = false;
        }
    }

    private void previewToolStripMenuItem_Click(object sender, EventArgs e)
    {
        MessageBox.Show("hi there!");
    }

    private void btnPreview_Click(object sender, EventArgs e)
    {
        try
        {
            if (lstHold.Items.Count <= 0)
            {
                MessageBox.Show("No pictures are available to preview");
            }
            else if (lstHold.SelectedItem == null)
            {
                MessageBox.Show("No picture is selected to preview");
            }
            else
            {
                MyPictures mp = (MyPictures)lstHold.SelectedItem;
                //Bitmap bmp = new Bitmap(mp.OldImgPath);
                Form2 frm = new Form2(mp);
                frm.Show();
            }
        }
        catch (Exception ex)
        {
            MessageBox.Show("An Error has occured:\n " + ex.ToString());
        }
    }
    public void ImgPropertiesEnabled()
    {
        //Enable Image properties when an image is selected
        if (lstHold.SelectedIndex >= 0)
        {
            gbCheckAll.Enabled = true;
            gbImgProperties.Enabled = true;
        }
        else
        {
            //Disable Image properties when an image is not selected
            gbCheckAll.Enabled = false;
            gbImgProperties.Enabled = false;

        }
        //Preview buttons enablement will depend on the same conditions
        btnPreview.Enabled = gbImgProperties.Enabled;
    }
    public void ClearTextBoxes()
    {
        txtNewImgPath.Text = "";
        txtNewName.Text = "";
        txtNewHeight.Text = Convert.ToString(0);
        txtNewWidth.Text = Convert.ToString(0);
        cbFormat.SelectedItem = null;
        chkSelectAll.Checked = false;
    }

}

Ответы [ 4 ]

7 голосов
/ 14 февраля 2010

Отсканировав код, да, это eleborate ... может быть, немного слишком;)

Одна вещь, которую я заметил, это ваши соглашения об именах. Хотя он ничего не меняет во время выполнения, он облегчает поддержку API / кода.

Итак, вместо того, чтобы иметь IPicture, я бы сделал что-то вроде `IResizableImage´ (читая вашу спецификацию, вот и все. Не просто картинка, а изменяемый размер) Вместо «ReturnImage ()» я бы использовал что-то вроде «Scale ()». 'ImageSave ()' в 'Save ()'

Ваш код начнет читать (Который добавил симметричную информацию в соответствии с соглашением об именах)

IResizableImage myImg = new ResizableImage( orignalBitmap );
Image rescaledImg = myImg.Scale( "new path", 320,240 );
resccaledImg.Save();

вместо:

IPicture myImg = new MyPictures();
Image rescaled = myImg.ReturnImage( "newpath", 320, 240 );
rescaledImg.ImageSave();

Итак, обычно классы - это существительные, методы - глаголы, прилагательные - свойства / поля. Постарайтесь минимизировать дублирование или дублирование. «ImageSave» - это метод для вашего изображения. Разве «Image.Save ()» не более понятен, чем «Image.ImageSave ()»?

Только некоторые из моих мыслей; В руководстве по кодированию нет абсолютно правильного или неправильного. Подумайте о том, чтобы быть другим человеком, когда ИСПОЛЬЗУЕТЕ API вместо ПИСЬМО API. Выйдите из поля «я знаю, что он делает» и представьте, что вы пользователь, никогда не видевший этот API раньше. Чувствуется ли это естественно и легко доступно?

Надеюсь, это поможет,

6 голосов
/ 14 февраля 2010

Вот некоторые улучшения для вашего кода и дизайна. Эти советы относятся не только к ОО, но вы должны знать, что хороший дизайн - это не просто ОО.

1. Избегайте комментирования очевидного.

//Declare variables to hold values that have been determined
private int _OldWidth;

Этот комментарий излишен, потому что любой программист поймет, что это декларация.

2. Избегайте неправильного указания имени. Например, класс «MyPictures» не совсем корректен, потому что:

  • Is содержит только одну картинку, а название предполагает множество картинок.
  • Он содержит «Мой», что, на мой взгляд, не правильно, так как если я читаю ваш код, это не мой класс. Это твое;)

3. Избегать объединения строк. Используйте string.Format или, для путей, Path.Combine

bmp.Save(_NewImgPath + @"\" + _NewImgName + "." +  _NewImgFormat.ToString().ToLower(), _NewImgFormat);

4. Сохраняйте методы короткими. Трудно держать все методы до 5 строк кода, но 30 строк (если мой счет правильный - без комментариев и пустых строк) для ProcessFiles - это слишком много.

5.Не используйте элементы дизайна только потому, что хотите их иметь. Я не вижу смысла использовать интерфейс в вашем коде. В вашем случае это только увеличивает сложность вашего кода. Более того, вы не использовали его ( !!! ). Вы только что реализовали это и все. Используйте интерфейсы, когда у вас есть несколько типов, которые совместно используют общие функции (те, что в интерфейсе), и вы хотите рассматривать их все как похожие, не зная о фактической реализации.

interface IImage
{
    void DrawLine(Point startPoint, Point endPoint);
}

class MonochromeImage:IImage
{
    void DrawLine(Point startPoint, Point endPoint)
    {
        //Draw a monochrome line on images with one channel
    }
}

class ColorImage:IImage
{
    void DrawLine(Point startPoint, Point endPoint)
    {
        //Draw a red line on images with three channels
    }
}

...

void DrawLineOnImage()
{
    List<IImage> images = new List<IImage>();
    images.Add(new ColorImage());
    images.Add(new MonochromeImage());

    //I am not aware of what kind of images actually have
    //all it matters is to have a draw line method
    foreach(IImage image in images)
    {
        image.DrawLine(p1,p2)
    }
}

6. Как уже упоминалось, попробуйте отделить представление (графический интерфейс пользователя - GUI) от логики. Сделайте так, чтобы вы могли заменить графический интерфейс без изменения логического кода.

7.Сделать функции единой ответственности. btnMoveOne_Click имеет более чем одну ответственность: он проверяет, существует ли файл, и обрабатывает элементы в пользовательском интерфейсе.

8.Ваш класс изображений связан с файловой системой. Что произойдет, если я захочу сохранить изображения, созданные в памяти? Какой путь тогда? Здесь вы можете улучшить дизайн класса. Сделайте так, чтобы не имело значения, находятся ли файлы с диска (HINT: в FileStream) или из памяти (HINT: в MemoryStream) или из любого другого места.

Пока это все. Надеюсь, эта информация поможет вам.

0 голосов
/ 14 февраля 2010

Ну, вот что я сделаю. Это, вероятно, отличается от того, что делают многие люди, но я думаю, что это довольно хороший, гибкий дизайн.

public abstract class AbstractConverter : IImageHandler
{
    public AbstractConverter(IImageHandler nextHandler)
    {
        output = nextHandler;
    }
    public void HandleImage(Bitmap b)
    {
        var outBitmap = Convert(b);
        output.HandleImage(outBitmap);
    }

    protected abstract Bitmap Convert(Bitmap input);

    private IImageHandler output;
}

public interface IImageHandler
{
    void HandleImage(Bitmap b);
}

Теперь остальная часть вашего приложения:

  1. Создание реализаций AbstractConverter для обработки отдельных преобразований, которые вы хотите
  2. Создание чего-то, что может собрать и соединить конвертеры вместе
  3. Получение начального растрового изображения и вывод окончательного результата.
0 голосов
/ 14 февраля 2010

Для достижения хорошего дизайна вам необходимо применить TDD (Test Driven Design).

Вскоре вы обнаружите, что для тестируемости требуется разделить проект на слои, такие как представление и бизнес-логика.

Начните покрывать ваш проект тестами, и вы не поверите, как быстро вы обнаружите несоответствия в дизайне.

Вещи просто встанут и будут кричать: "Ни за что ты не будешь меня испытывать!" Наихудшим анемом является код, скрытый в WinForms. Что вы можете сделать, так это сделать представление «скромным». http://codebetter.com/blogs/jeremy.miller/archive/2007/05/23/build-your-own-cab-part-2-the-humble-dialog-box.aspx

Что касается примеров проекта, вы должны смотреть на архитектурные шаблоны, а не на образцы ООП. Ключевые слова, по которым вы будете искать: MVC, MVP, MVVM.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...