[RexGrammer]

I started a new application, a signature maker (took the idea of martyr2's idea list).

So it has a text input, you can select the font, the color and the location.
You can select a background image from a listbox (to be implemented).

So if you can review this application, I would be gratefull. (Sorry if I'm boring with all this 'review my program' topics , but I'm learning and I need to know if I'm heading in the correct direction)

This is a very EARLY ALPHA version. So please don't be too harsh.

Signature Maker.

Thanks!

This post has been edited by RexGrammer: 15 December 2011 - 01:51 PM

[janne_panne]

There is hardly any code to review, just like 30 lines or something
How about you make the software ready and then ask for review instead of early alpha version which takes 30 minutes for senior programmer to make and few hours for beginner.

But I'll give it a shot anyway since I don't want to be rude:

1. Don't write code inside methods which are used for handling events, create methods instead. This will improve code re-usability. What I mean is, convert this:

        private void mainForm_Load(object sender, EventArgs e)
        {
            //Foreach string (that holds the image path in the paths
            foreach (string image in GetImagesPath())
            {
                //Add that item to the backgroundList
                backgroundsList.Items.Add(Path.GetFileNameWithoutExtension(image));
            }
        }

into this:

        private void mainForm_Load(object sender, EventArgs e)
        {
            LoadImages();
        }

        private void LoadImages()
        {
            //Foreach string (that holds the image path in the paths
            foreach (string image in GetImagesPath())
            {
                //Add that item to the backgroundList
                backgroundsList.Items.Add(Path.GetFileNameWithoutExtension(image));
            }
        }

Now you could call LoadImages from other methods too, instead of just mainForm_Load. And calling mainForm_Load from another method like you could call LoadImages now wouldn't be a good programming.

2. Try to divide your code into layers. Read about n-tier architecture. This will also help at code re-usability.

For example you could create a class library which is meant for creating signature on images. Or maybe you could start a class library for all kinds of image handling and at first there is only this signature thing.

Important thing in that class library would be that it must not have any user interface references in it. This way the user of the library could create an user interface he wants and just call your class library's methods to add the signature on the image.

If it sounds too complicated and you don't know what to do, you could start at baby steps and convert your methods to not reference any exact UI elements. In the following example I have removed all references to previewBox and added the Image in parameters instead:

        private void UpdateSignature(Image image, string text, Font font, Brush brush, int x, int y)
        {
            //Create new graphics 
            Graphics g = Graphics.FromImage(image);
            //Draw the text
            g.DrawString(text, font, brush, (float)x, (float)y);
        }

        private void SaveImage(Image image, string path, ImageFormat format)
        {
            using (Stream stream = new FileStream(path, FileMode.Create))
                image.Save(stream, format);
        }

Now where you call the methods, just pass in the previewBox.BackgroundImage. Now the method isn't bound to the exact previewBox, you can pass any Image to it. Even in console application you could add a signature on an image.

[RexGrammer]

Thanks for that this app was just a test of concepts, I was looking for that kind of review, how much this is OO and such... So thanks