So a quick update on my Clean Code book challenge. I’m starting on chapter 5 this evening. The first four chapters covered variable naming, function naming, code commenting, and generally what is clean code. Chapter five looks to structure and format code and I’m looking forward to that. Although I have to wonder if this topic is dated as most modern IDE’s automatically structure and format code blocks, so we’ll see how it goes.
I’m really enjoying the topics and while I’m reading it I’m reflecting on code I’ve written in the past. This reflection has made me think of my programming skills and a general timeline of how much I’ve progressed in just this past year. A year ago I knew nothing about design patterns, my understanding of polymorphism was limited at best and most of my code was tightly coupled and used singleton patterns throughout. All terrible design decisions for object oriented coding.
Here’s an example of some bad code I found, while I haven’t refactored this code yet as I have bigger plans for the project that this code resides in. I think highlighting at-least some of terrible decisions I made, reinforces the topics learned in Clean Code
/*------------------------------ POPULATE BLE WITH ADV DATA -------------------------------*/ -(void)populateData:(NSDictionary *)advertisement :(CBPeripheral *)peripheral :(NSNumber *)RSSI { NSData *rawData = [advertisement objectForKey:@"kCBAdvDataManufacturerData"]; [rawData getBytes:&myData length:sizeof(myData)]; if(myData.compId == 7057) { NSLog(@"POPULATE DATA OBJECT"); _compId = myData.compId; // Company Identifier _deviceName = [peripheral name]; // Device Name _temp = myData.temperature / 100.0; // temperature divided by 100 to return xx.x format _rampRate = myData.rampRate; // How quickly the temp is rising _dutyFactor = myData.dutyFactor; // PLACE HOLDER _flagStatus = myData.statusFlag; // Device Status _speedRPM = myData.speedRpm; // Device revolutions per minute _vibration = myData.vib; // Device Vibration _radioStatus = myData.radioStatus; // BLE Radio status _cycleTimer = myData.cycleTimer; // PLACE HOLDER _batteryLevel = myData.batteryLevel; // Battery level _currentAlarm = myData.alarms; _macAddress = [Utility getMacAddress:myData.mac]; // Device MAC Address _lastUpdate = [NSDate date]; // Last time the device sent an advertisement packet NSLog(@"current alarm packet: %hu",_currentAlarm); if([RSSI isEqual: @127]) { // Update RSSI if value is NOT 127 NSLog(@"127 BUG"); NSLog(@"RSSI: %@ RSSI PREVIOUS: %@",RSSI,rssiPrevious); _rssi = rssiPrevious; } else { _rssi = RSSI; rssiPrevious = RSSI; } _hasAuthorized = false; _updateTimer = [NSTimer scheduledTimerWithTimeInterval:.5 target:self selector:@selector(updateLastUpdateTime) userInfo:nil repeats:YES]; _isConnectable = [[advertisement objectForKey:CBAdvertisementDataIsConnectable] boolValue]; } }
If you’ve read this far and still don’t know what this code is doing, let me help. Its taking a bluetooth low energy Advertisement packet, dropping it into a data structure. The code first of all takes too many arguments, as pointed out by Robert Martin, 3 arguments is too many. Code comments create excessive noise, naming convention on the method itself is obscure and not capitalized. The log statements though-out create more noise. Overall the entire code block is too loo long as well. I also dislike the constants that are defined within the method itself, for example the 7057 is a manufacturer id, what if they change their ID? this entire method would break.
Anyway, if you haven’t started reading this book yet please do. It’s very enlightening, I’m learning a ton.
I’ll see you guys tomorrow when we continue our journey down C# and discuss methods.
Happy Coding,
Corey
This post first appeared on Diaonic - Coding, Tutorials, And Life Hacks, please read the originial post: here