마시남

마시남

마시남의 블로그 입니다.

[Refactoring/C#] C# 코드 리팩터링 하기

Date:     Updated:

카테고리:

태그:

이번에 블루투스 저전력을 C#으로 구현하면서, 문서에 있는 코드를 그대로 옮기니 코드가 너무 지저분해지는것을 느꼈습니다. 해당 코드를 리팩터링하는 과정을 기술하였습니다.

리팩터링

우선, 원본 코드 입니다.

using System.Reflection.PortableExecutable;
using Windows.Devices.Bluetooth;
using Windows.Devices.Bluetooth.GenericAttributeProfile;
using Windows.Devices.Enumeration;
using Windows.Storage.Streams;


namespace MotionMaestro_ver._1.Forms
{
    public partial class Form1 : Form
    {
        private DeviceWatcher deviceWatcher;
        private DeviceInformation selectedDevice;
        private BluetoothLEDevice bluetoothLeDevice;
        private GattCharacteristic notifyCharacteristic;

        public Form1()
        {
            InitializeComponent();
        }

        private void StartDeviceWatcher(object sender, EventArgs e)
        {
            string[] requestedProperties = { "System.Devices.Aep.DeviceAddress", "System.Devices.Aep.IsConnected" };

            deviceWatcher = DeviceInformation.CreateWatcher(
                BluetoothLEDevice.GetDeviceSelectorFromPairingState(false),//페어링 되어 있는가 없는가 판단
                requestedProperties,
                DeviceInformationKind.AssociationEndpoint);

            deviceWatcher.Added += DeviceWatcher_Added;
            deviceWatcher.Updated += DeviceWatcher_Updated;
            deviceWatcher.Removed += DeviceWatcher_Removed;
            deviceWatcher.EnumerationCompleted += DeviceWatcher_EnumerationCompleted;
            deviceWatcher.Stopped += DeviceWatcher_Stopped;

            deviceWatcher.Start();
        }

        private async void DeviceWatcher_Added(DeviceWatcher sender, DeviceInformation deviceInfo)
        {
            Invoke(new Action(() =>
            {
                textBox1.AppendText(deviceInfo.Name);
            }));

            if (deviceInfo.Name == "Heart Monitor")
            {
                selectedDevice = deviceInfo;
                deviceWatcher.Stop();

                bluetoothLeDevice = await BluetoothLEDevice.FromIdAsync(selectedDevice.Id);
                if (bluetoothLeDevice != null)
                {
                    this.Invoke(new Action(() =>
                    {
                        textBox1.AppendText("Device connected!\n");
                    }));

                    await DiscoverServices();
                }
                else
                {
                    textBox1.AppendText("Failed to connect to device.\n");
                }
            }
        }

        private async Task DiscoverServices()
        {
            var servicesResult = await bluetoothLeDevice.GetGattServicesAsync();

            if (servicesResult.Status == GattCommunicationStatus.Success)
            {
                foreach (var service in servicesResult.Services)
                {
                    string serviceUuid = service.Uuid.ToString();

                    if (serviceUuid.Equals("0000180D-0000-1000-8000-00805F9B34FB", StringComparison.OrdinalIgnoreCase))
                    {
                        var characteristicsResult = await service.GetCharacteristicsAsync();

                        foreach (var characteristic in characteristicsResult.Characteristics)
                        {
                            string characteristicUuid = characteristic.Uuid.ToString();

                            bool hasNotifyFlag = characteristic.CharacteristicProperties.HasFlag(GattCharacteristicProperties.Notify);
                            bool targetCharacteristic = characteristicUuid.Equals("00002A37-0000-1000-8000-00805F9B34FB", StringComparison.OrdinalIgnoreCase);
                            if (hasNotifyFlag && targetCharacteristic)
                            {
                                notifyCharacteristic = characteristic;
                                notifyCharacteristic.ValueChanged += NotifyCharacteristic_ValueChanged;

                                try
                                {
                                    await notifyCharacteristic.WriteClientCharacteristicConfigurationDescriptorAsync(
                                        GattClientCharacteristicConfigurationDescriptorValue.Notify);

                                    this.Invoke(new Action(() =>
                                    {
                                        textBox1.AppendText("notification Enabled\n");
                                    }));

                                    break;
                                }
                                catch (Exception ex)
                                {
                                    this.Invoke(new Action(() =>
                                    {
                                        textBox1.AppendText("실패 : " + ex.Message + "\n");
                                    }));
                                }
                            }
                        }
                    }
                }
            }
            else
            {
                MessageBox.Show("Failed to discover GATT services.");
            }
        }


        private void NotifyCharacteristic_ValueChanged(GattCharacteristic sender, GattValueChangedEventArgs args)
        {
            var reader = DataReader.FromBuffer(args.CharacteristicValue);
            byte[] input = new byte[args.CharacteristicValue.Length];
            reader.ReadBytes(input);
            string received = System.Text.Encoding.UTF8.GetString(input);

            Invoke(new Action(() =>
            {
                textBox1.AppendText($"Received: {received}\n");
            }));
        }

        private void DeviceWatcher_Updated(DeviceWatcher sender, DeviceInformationUpdate args)
        {
            // 필요에 따라 업데이트 처리
            this.Invoke(new Action(() =>
            {
                textBox1.AppendText("Updated\n");
            }));
        }

        private void DeviceWatcher_Removed(DeviceWatcher sender, DeviceInformationUpdate args)
        {
            WorkOnUiThread(() => textBox1.AppendText("Removed\n"));
        }

        private void DeviceWatcher_EnumerationCompleted(DeviceWatcher sender, object args)
        {
            this.Invoke(new Action(() =>
            {
                textBox1.AppendText("Device enumeration completed.\n");
            }));
        }

        private void DeviceWatcher_Stopped(DeviceWatcher sender, object args)
        {
            WorkOnUiThread(() => textBox1.AppendText("Stopped"));
        }

        private void WorkOnUiThread(Action _action)
        {
            this.Invoke(_action);
        }
    }
}

우선, 가장 복잡해 보이는 부분은 DiscoverServices 함수 입니다. 이 부분이 가장 복잡해 보이고, 가장 중요한 부분입니다. 이 부분을 먼저 리팩터링 해봅니다.

우선, if문과 for 문들이 중첩되어있어 흐름을 한번에 이해하기 어렵습니다. 이것을 반복문 추출하기를 통해 추출 합니다.

반복문 추출하기

DiscoverServices는 다음과 같은 순서를 따릅니다.

  1. 블루투스 디바이스 찾기
  2. 디바이스의 Serivce들을 순회하며 타겟 Service를 찾기
  3. 타겟 서비스안의 타겟 Characteristic을 찾기
  4. 타겟 Characterisitc 이 알림 구독 가능한지 확인하기

위 순서대로 함수를 나눠서 리팩터링 합니다. 우선, 디바이스의 Service들을 순회하며 타겟 Service를 찾는 부분부터 추출 합니다. 함수의 이름을 GetTargetService로 짓겠습니다.

private GattDeviceService? GetTargetService(IReadOnlyList<GattDeviceService> _services)
{
    GattDeviceService? result = null;

    foreach (var service in _services)
    {
        string serviceUuid = service.Uuid.ToString();

        if (serviceUuid.Equals("0000180D-0000-1000-8000-00805F9B34FB", StringComparison.OrdinalIgnoreCase))
        {
            result = service;
        }
    }
    return result;
}

위와같은 함수가 만들어 졌습니다.

마찬가지로, 타겟 Characteristic을 구하는 부분도 반복문 추출하기를 통해 함수로 만들어 줍니다.

 private async Task<GattCharacteristic?>? GetTargetCharacteristic(GattDeviceService _service)
 {
     GattCharacteristic? result = null;

     var characteristicsResult = await _service.GetCharacteristicsAsync();

     foreach (var characteristic in characteristicsResult.Characteristics)
     {
         string characteristicUuid = characteristic.Uuid.ToString();

         if (characteristicUuid.Equals("00002A37-0000-1000-8000-00805F9B34FB", StringComparison.OrdinalIgnoreCase))
         {
             result = characteristic;
         }
     }

     return result;
 }

여기까지 작성을 했습니다. 지금 보니 또 불편한것이 생겼습니다. UUID 들이 참조형식이 아닌 값형식으로 들어가있어서 관리가 힘들고, C#이 제공하는 LINQ를 사용할 수 있을것만 같습니다.

클래스 추출하기 & 반복문을 파이프라인으로 바꾸기

클래스 추출하기와 반복문을 파이프라인으로 바꾸기를 통해서 리팩터링 해줍니다.

namespace BluetoothLibrary.BLE
{
    public class BleDeviceConfig
    {
        public BleDeviceConfig(string _deviceName, string _targetUuid, string _targetCharacteristicUuid)
        {
            this.DeviceName = _deviceName;
            this.TargetUuid = _targetUuid;
            this.TargetCharacteristicUuid = _targetCharacteristicUuid;
        }

        public string DeviceName { get; set; }
        public string TargetUuid { get; set; }
        public string TargetCharacteristicUuid { get; set; }
    }
}

private GattDeviceService? GetTargetService(IReadOnlyList<GattDeviceService> _services)
{
	return _services.FirstOrDefault(service => service.Uuid.ToString().Equals(deviceConfig.TargetUuid, StringComparison.OrdinalIgnoreCase));
}

private async Task<GattCharacteristic?>? GetTargetCharacteristic(GattDeviceService _service)
{
	var characteristics = await _service.GetCharacteristicsAsync();
	var characteristicsList = characteristics.Characteristics;

	return characteristicsList.FirstOrDefault(characteristics => characteristics.Uuid.ToString().Equals(deviceConfig.TargetCharacteristicUuid, StringComparison.OrdinalIgnoreCase));
}

그리고, 알림구독을 할 수 있는지 판단하는 로직도 함수로 추출해 줍니다. 또한, 필요없어진 전역변수 notifyCharacteristic 을 제거해줍니다.

private async Task EnableNotificationsAsync(GattCharacteristic characteristic)
{
	bool hasNotifyFlag = characteristic.CharacteristicProperties.HasFlag(GattCharacteristicProperties.Notify);

	if (hasNotifyFlag)
	{
		characteristic.ValueChanged += NotifyCharacteristic_ValueChanged;
		try
		{
			await characteristic.WriteClientCharacteristicConfigurationDescriptorAsync(GattClientCharacteristicConfigurationDescriptorValue.Notify);
		}
		catch (Exception ex)
		{
			RunOnUiThread(() => textBox1.AppendText(ex.Message));
		}
	}
}

함수 추출하기

그리고, 함수 로직들 중간에 보이는 로깅용 TextBox를 업데이트하는 부분이 있습니다. 지금 async를 통해 UI 스레드가 아닌 다른 스레드에서 작업을 하기 때문에 Invoke로 작업을 시키고 있습니다. 해당 부분도 반복되니, 함수로 추출 합니다.

private void RunOnUiThread(Action _action)
{
	this.Invoke(_action);
}

여기까지 하고, DiscoverServices를 재구성 해보겠습니다.

private async Task DiscoverServices()
{
    var servicesResult = await bluetoothLeDevice.GetGattServicesAsync();

    if (servicesResult.Status == GattCommunicationStatus.Success)
    {
        GattDeviceService? targetService = GetTargetService(servicesResult.Services);
        if (targetService == null)
        {
            return;
        }

        GattCharacteristic? targetChracteristic = await GetTargetCharacteristic(targetService);
        if (targetChracteristic == null)
        {
            return;
        }

        await EnableNotificationsAsync(targetChracteristic);
    }
    else
    {
        MessageBox.Show("Failed to discover GATT services.");
    }
}

함수명 바꾸기

이렇게 놓고 보니, if문도 거슬리는군요. 그리고, 처음 지었던 함수 이름이 마음에 들지 않습니다. DiscoverService 에서 ConnectToTargetDevice로 이름을 바꿔 줍니다.

private async Task ConnectToTargetDevice()
{
	try
	{
		var servicesResult = await bluetoothLeDevice.GetGattServicesAsync();

		if (servicesResult.Status != GattCommunicationStatus.Success)
		{
			MessageBox.Show("Failed To discover GATT services");
			return;
		}

		GattDeviceService? targetService = GetTargetService(servicesResult.Services);
		if (targetService == null)
		{
			return;
		}

		GattCharacteristic? targetChracteristic = await GetTargetCharacteristic(targetService);
		if (targetChracteristic == null)
		{
			return;
		}

		await EnableNotificationsAsync(targetChracteristic);
	}
	catch (Exception ex)
	{
		RunOnUiThread(() => textBox1.AppendText(ex.Message));
	}
}

위과 같이 리팩터링하니, 훨씬 깔끔해졌습니다.

함수 추출하기

다음으로 거슬리는 부분은 DeviceWatcher_Added 부분 입니다. 여기서 디바이스를 찾는 과정을 함수로 나타내고 싶습니다.

private async void DeviceWatcher_Added(DeviceWatcher sender, DeviceInformation deviceInfo)
{
    RunOnUiThread(() => textBox1.AppendText(deviceInfo.Name));

    if (deviceInfo.Name == "Heart Monitor")
    {
        selectedDevice = deviceInfo;
        deviceWatcher.Stop();

        bluetoothLeDevice = await BluetoothLEDevice.FromIdAsync(selectedDevice.Id);
        if (bluetoothLeDevice != null)
        {
            RunOnUiThread(() => textBox1.AppendText("Device connected!\n"));

            await ConnectToTargetDevice();
        }
        else
        {
            textBox1.AppendText("Failed to connect to device.\n");
        }
    }
}

bluetoothDevice 의 정보를 가져오는 부분을 함수로 추출 합니다. 그리고, 필요없는 전역변수인 selectedDevice와 bleutoothLEDevice를 삭제 합니다.

private async void DeviceWatcher_Added(DeviceWatcher sender, DeviceInformation deviceInfo)
{
    RunOnUiThread(() => textBox1.AppendText(deviceInfo.Name));

    if (deviceInfo.Name == "Heart Monitor")
    {
        deviceWatcher.Stop();
        BluetoothLEDevice bluetoothLEDevice = await GetLEDevice(deviceInfo);

        if (bluetoothLEDevice != null)
        {
            RunOnUiThread(() => textBox1.AppendText("Device connected!\n"));

            await ConnectToTargetDevice(bluetoothLEDevice);
        }
        else
        {
            textBox1.AppendText("Failed to connect to device.\n");
        }
    }
}

또 다음으로 거슬리는 부분은 데이터를 읽는 부분입니다. 데이터를 읽는 부분의 로직이 어디까지인지 해석해야한다는 불편함이 있습니다. 이부분도 함수 추출하기를 통해 함수로 만들어 줍니다.

private void NotifyCharacteristic_ValueChanged(GattCharacteristic sender, GattValueChangedEventArgs args)
{
	var reader = DataReader.FromBuffer(args.CharacteristicValue);
	byte[] input = new byte[args.CharacteristicValue.Length];
	reader.ReadBytes(input);
	string received = System.Text.Encoding.UTF8.GetString(input);

	RunOnUiThread(() => textBox1.AppendText($"Received: {received}\n"));
}

private void NotifyCharacteristic_ValueChanged(GattCharacteristic sender, GattValueChangedEventArgs args)
{
    string recivedData = ReadRecivedData(args, Encoding.UTF8);

    RunOnUiThread(() => textBox1.AppendText($"Received: {recivedData}\n"));
}

private string ReadRecivedData(GattValueChangedEventArgs _args, Encoding _encoding)
{
    var reader = DataReader.FromBuffer(_args.CharacteristicValue);
    byte[] input = new byte[_args.CharacteristicValue.Length];
    reader.ReadBytes(input);
    return _encoding.GetString(input);
}

마지막으로, StartDeviceWatcher이 부분에서, DeviceWatcher를 설정하는 부분과, DeviceWatcher를 시작하는 부분이 함께 있으므로, 함수 추출하기를 통해 함수로 나타내어 줍니다.

private void StartDeviceWatcher(object sender, EventArgs e)
{
	string[] requestedProperties = { "System.Devices.Aep.DeviceAddress", "System.Devices.Aep.IsConnected" };

	deviceWatcher = DeviceInformation.CreateWatcher(
		BluetoothLEDevice.GetDeviceSelectorFromPairingState(false),//페어링 되어 있는가 없는가 판단
		requestedProperties,
		DeviceInformationKind.AssociationEndpoint);

	deviceWatcher.Added += DeviceWatcher_Added;
	deviceWatcher.Updated += DeviceWatcher_Updated;
	deviceWatcher.Removed += DeviceWatcher_Removed;
	deviceWatcher.EnumerationCompleted += DeviceWatcher_EnumerationCompleted;
	deviceWatcher.Stopped += DeviceWatcher_Stopped;

	deviceWatcher.Start();
}

private void StartDeviceWatcher(object sender, EventArgs e)
{
    InitDeviceWatcher();

    deviceWatcher.Start();
}

private void InitDeviceWatcher()
{
    string[] requestedProperties = { "System.Devices.Aep.DeviceAddress", "System.Devices.Aep.IsConnected" };

    deviceWatcher = DeviceInformation.CreateWatcher(
        BluetoothLEDevice.GetDeviceSelectorFromPairingState(false),//페어링 되어 있는가 없는가 판단
        requestedProperties,
        DeviceInformationKind.AssociationEndpoint);

    deviceWatcher.Added += DeviceWatcher_Added;
    deviceWatcher.Updated += DeviceWatcher_Updated;
    deviceWatcher.Removed += DeviceWatcher_Removed;
    deviceWatcher.EnumerationCompleted += DeviceWatcher_EnumerationCompleted;
    deviceWatcher.Stopped += DeviceWatcher_Stopped;
}

마무리

마지막으로, Public 메서드와 Event 메서드와 Private 메서드끼리 모아주면, 리팩터링이 끝나게 됩니다.

namespace MotionMaestro_ver._1.Forms
{
    public partial class Form1 : Form
    {
        private DeviceWatcher deviceWatcher;
        private BleDeviceConfig deviceConfig;

        public Form1()
        {
            InitializeComponent();
            deviceConfig = new BleDeviceConfig("Heart Monitor", "0000180D-0000-1000-8000-00805F9B34FB", "00002A37-0000-1000-8000-00805F9B34FB");
        }

        #region Events
        private void StartDeviceWatcher(object sender, EventArgs e)
        {
            InitDeviceWatcher();
            deviceWatcher.Start();
        }
        private async void DeviceWatcher_Added(DeviceWatcher sender, DeviceInformation deviceInfo)
        {
            RunOnUiThread(() => textBox1.AppendText(deviceInfo.Name));

            if (deviceInfo.Name == "Heart Monitor")
            {
                deviceWatcher.Stop();
                BluetoothLEDevice bluetoothLEDevice = await GetLEDevice(deviceInfo);

                if (bluetoothLEDevice != null)
                {
                    RunOnUiThread(() => textBox1.AppendText("Device connected!\n"));

                    await ConnectToTargetDevice(bluetoothLEDevice);
                }
                else
                {
                    textBox1.AppendText("Failed to connect to device.\n");
                }
            }
        }
        private void NotifyCharacteristic_ValueChanged(GattCharacteristic sender, GattValueChangedEventArgs args)
        {
            string recivedData = ReadRecivedData(args, Encoding.UTF8);

            RunOnUiThread(() => textBox1.AppendText($"Received: {recivedData}\n"));
        }

        private void DeviceWatcher_Updated(DeviceWatcher sender, DeviceInformationUpdate args)
        {
            RunOnUiThread(() => textBox1.AppendText("Updated\n"));
        }

        private void DeviceWatcher_Removed(DeviceWatcher sender, DeviceInformationUpdate args)
        {
            RunOnUiThread(() => textBox1.AppendText("Removed\n"));
        }

        private void DeviceWatcher_EnumerationCompleted(DeviceWatcher sender, object args)
        {
            RunOnUiThread(() => textBox1.AppendText("Device enumeration completed. \n"));
        }

        private void DeviceWatcher_Stopped(DeviceWatcher sender, object args)
        {
            RunOnUiThread(() => textBox1.AppendText("Stopped"));
        }
        #endregion

        #region Private Methods
        private void InitDeviceWatcher()
        {
            string[] requestedProperties = { "System.Devices.Aep.DeviceAddress", "System.Devices.Aep.IsConnected" };

            deviceWatcher = DeviceInformation.CreateWatcher(
                BluetoothLEDevice.GetDeviceSelectorFromPairingState(false),//페어링 되어 있는가 없는가 판단
                requestedProperties,
                DeviceInformationKind.AssociationEndpoint);

            deviceWatcher.Added += DeviceWatcher_Added;
            deviceWatcher.Updated += DeviceWatcher_Updated;
            deviceWatcher.Removed += DeviceWatcher_Removed;
            deviceWatcher.EnumerationCompleted += DeviceWatcher_EnumerationCompleted;
            deviceWatcher.Stopped += DeviceWatcher_Stopped;
        }

        private async Task<BluetoothLEDevice> GetLEDevice(DeviceInformation _device)
        {
            return await BluetoothLEDevice.FromIdAsync(_device.Id);
        }

        private async Task ConnectToTargetDevice(BluetoothLEDevice _bluetoothLEDevice)
        {
            try
            {
                var servicesResult = await _bluetoothLEDevice.GetGattServicesAsync();

                if (servicesResult.Status != GattCommunicationStatus.Success)
                {
                    MessageBox.Show("Failed To discover GATT services");
                    return;
                }

                GattDeviceService? targetService = GetTargetService(servicesResult.Services);
                if (targetService == null)
                {
                    return;
                }

                GattCharacteristic? targetChracteristic = await GetTargetCharacteristic(targetService);
                if (targetChracteristic == null)
                {
                    return;
                }

                await EnableNotificationsAsync(targetChracteristic);
            }
            catch (Exception ex)
            {
                RunOnUiThread(() => textBox1.AppendText(ex.Message));
            }
        }

        private GattDeviceService? GetTargetService(IReadOnlyList<GattDeviceService> _services)
        {
            return _services.FirstOrDefault(service => service.Uuid.ToString().Equals(deviceConfig.TargetUuid, StringComparison.OrdinalIgnoreCase));
        }

        private async Task<GattCharacteristic?>? GetTargetCharacteristic(GattDeviceService _service)
        {
            var characteristics = await _service.GetCharacteristicsAsync();
            var characteristicsList = characteristics.Characteristics;

            return characteristicsList.FirstOrDefault(characteristics => characteristics.Uuid.ToString().Equals(deviceConfig.TargetCharacteristicUuid, StringComparison.OrdinalIgnoreCase));
        }

        private async Task EnableNotificationsAsync(GattCharacteristic characteristic)
        {
            bool hasNotifyFlag = characteristic.CharacteristicProperties.HasFlag(GattCharacteristicProperties.Notify);

            if (hasNotifyFlag)
            {
                characteristic.ValueChanged += NotifyCharacteristic_ValueChanged;
                try
                {
                    await characteristic.WriteClientCharacteristicConfigurationDescriptorAsync(GattClientCharacteristicConfigurationDescriptorValue.Notify);
                }
                catch (Exception ex)
                {
                    RunOnUiThread(() => textBox1.AppendText(ex.Message));
                }
            }
        }

        private string ReadRecivedData(GattValueChangedEventArgs _args, Encoding _encoding)
        {
            var reader = DataReader.FromBuffer(_args.CharacteristicValue);
            byte[] input = new byte[_args.CharacteristicValue.Length];
            reader.ReadBytes(input);
            return _encoding.GetString(input);
        }

        private void RunOnUiThread(Action _action)
        {
            this.Invoke(_action);
        }
        #endregion
    }
}

처음보다 코드의 Depth도 많이 줄어들었고, 각 기능을 담당하는 함수를 만들어 디버깅도 쉽게 하였습니다. 그리고 리팩터링 책에서 강조하는것 바로 테스트 하기 입니다. 지금의 포스팅에는 테스트가 생략되어 있지만, 리팩터링을 할 때 테스트하기는 필수이니 꼭 해야 합니다.

그리고, 지금은 컨텍스트에 블루투스가 그대로 들어가있는 상태입니다. 해당 코드를 기능이동을 통해 라이브러리화 하여 사용하면 더 좋은 코드가 만들어 질것입니다. 라이브러리를 만드는 과정은 다른 포스팅에서 하도록 하겠습니다.

Refactoring 카테고리 내 다른 글 보러가기

댓글 남기기

최근 글 10 개 :)