Simple image capture program

Question

This program is eventually going to be a screen recorder, but right now I just want it to capture screen images.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;
using System.Drawing;
using System.Windows.Forms;


namespace ScreenRecorder
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {
        public static bool recording = false;
        public static string saveAddress = @"C:\Users\UserName\Desktop\something.bmp";

        public MainWindow()
        {
            InitializeComponent();
        }

        private void onClick(object sender, RoutedEventArgs e)
        {
            System.Drawing.Image currentImage; 
            if (recording == false)
            {
                currentImage =  this.CaptureScreen();
                recording = true;
                recorderButton.Content = "Stop Recording!";
                currentImage.Save(saveAddress);
            }
            else
            {
                recording = false;
                recorderButton.Content = "Start Recording!";
            }
        }

        private System.Drawing.Image CaptureScreen()
        {
            System.Drawing.Rectangle screenSize = Screen.PrimaryScreen.Bounds;
            Bitmap target = new Bitmap(screenSize.Width, screenSize.Height);
            using (Graphics g = Graphics.FromImage(target))
            {
                g.CopyFromScreen(0, 0, 0, 0, new System.Drawing.Size(screenSize.Width, screenSize.Height));
            }
            return target;
        }
    }
}

Answer 1

Considering there is almost no code to review, this will be purely stylistic. Remember that criticism is a sign of love.

/// <summary>
/// Interaction logic for MainWindow.xaml
/// </summary>

• The comment isn't needed, we all know what a codebehind is for. Comments should explain the why, not the what.

• You should (almost) never have public fields. If they are actually needed outside of the current object's scope then you should make them properties, otherwise private.

• Are they consciously static? This will mess up your program if you execute it multiple times simultaneously. A static modifier on a field is something you probably won't need very often and should be avoided unless needed.

• You're hardcoding the user's path. It would be more userfriendly to let them pick this through a FolderBrowserDialog.

• C#'s naming conventions dictate that methods are UpperCamelCase, not lowerCamelCase.

• You've already imported System.Drawing, remove all the explicit namespace naming in your code.

• currentImage is only used inside the first part of your if block so you should declare it there. Always restrict the visibility of variables to the most-restrictive scope possible. You'll notice you won't even need the temporary variable and might as well use CaptureScreen().Save(saveAddress).

• if(recording == false) can be less verbosely written as if(!recording).

• Don't use this unless you need it to distinguish between members of different scopes.

• Use resource files to store your buttons their text so you can easily re-use messages and add additional languages. It also allows you to centrally manage all messages.

• CaptureScreen() indicates a long-running operation with perhaps events raised to return information. That's not the case here. I suggest naming it GetScreenCapture().

• Some people prefer using var to - again - be less verbose.

Answer 2

Eventhandler

The name onClick() is better suited for a method raising an event than for an eventhandler. Usually if your object has an event, you will have a pattern like

public event EventHandler EventName;
protected virtual void OnEventName(EventArgs e)
{
    EventHandler handler = EventName;
    if (handler != null)
    {
        handler(this, e);
    }
}

Unneeded code

This program is eventually going to be a screen recorder, but right now I just want it to capture screen images.

Then you should remove any code which doesn't belong here.

Right now, after the first screen capturing you need to click the button 2 times to get one screenshot.

Simplification

You don't need to create a new Size object. You can either use the target.Size or screenSize.Size property for calling g.CopyFromScreen(). Additional you better create a Size instead of a Rectangle.

Refactoring

    public MainWindow()
    {
        InitializeComponent();
        recorderButton.Content = "Make and save screenshot";
    }

    private void onClick(object sender, RoutedEventArgs e)
    {
        System.Drawing.Image currentImage = CaptureScreen();
        currentImage.Save(saveAddress);
    }   

    private System.Drawing.Image CaptureScreen()
    {
        Size screenSize = Screen.PrimaryScreen.Bounds.Size;
        Bitmap target = new Bitmap(screenSize.Width, screenSize.Height);
        using (Graphics g = Graphics.FromImage(target))
        {
            g.CopyFromScreen(0, 0, 0, 0, screenSize);
        }
        return target;
    }

this looks much cleaner this way. You shouldn't start to implement features you might need in the future.