930 likes | 1.09k Views
Extreme Coding : Take control of your code. Exilesoft Johannes Brodwall Exilesoft Chief scientist @ jhannes. TODO: Preparator refactoring of MyTime. Are you in control of the code ?. Or is the code in control of you ?. Kata New code Old code. Part I.
E N D
Extreme Coding:Takecontrolofyourcode Exilesoft Johannes Brodwall Exilesoft Chief scientist @jhannes TODO: Preparator refactoring of MyTime
Kata New code Old code
publicclassPrimeFactorsTest { @Test publicvoidoneHasNoFactors() { assertTrue(getPrimeFactors(1).isEmpty()); } } Think of the simplest test case
publicclassPrimeFactorsTest { @Test publicvoidoneHasNoFactors() { assertTrue(getPrimeFactors(1).isEmpty()); } private List<Integer> getPrimeFactors(inti) { // TODO Auto-generated method stub returnnull; } } Make the code compile => Test runs red
publicclassPrimeFactorsTest { @Test publicvoidoneHasNoFactors() { assertTrue(getPrimeFactors(1).isEmpty()); } private List<Integer> getPrimeFactors(inti) { returnnewArrayList<>(); } } The simplest thing to green
publicclassPrimeFactorsTest { @Test publicvoidoneHasNoFactors() { assertTrue(getPrimeFactors(1).isEmpty()); } private List<Integer> getPrimeFactors(inti) { List<Integer> factors = newArrayList<>(); return factors; } } Refactor
publicclassPrimeFactorsTest { @Test publicvoidoneHasNoFactors() { assertTrue(getPrimeFactors(1).isEmpty()); } @Test publicvoidfactorsOfTwo() { assertEquals(Arrays.asList(2), getPrimeFactors(2)); } private List<Integer> getPrimeFactors(inti) { List<Integer> factors = newArrayList<>(); return factors; } } The next simplest test => Tests fail
publicclassPrimeFactorsTest { @Test publicvoidoneHasNoFactors() { assertTrue(getPrimeFactors(1).isEmpty()); } @Test publicvoidfactorsOfTwo() { assertEquals(Arrays.asList(2), getPrimeFactors(2)); } private List<Integer> getPrimeFactors(inti) { List<Integer> factors = newArrayList<>(); if (i == 2) factors.add(2); return factors; } } Simplest possible thing: Special case it
publicclassPrimeFactorsTest { @Test publicvoidoneHasNoFactors() { assertTrue(getPrimeFactors(1).isEmpty()); } @Test publicvoidfactorsOfTwo() { assertEquals(Arrays.asList(2), getPrimeFactors(2)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); if (number == 2) factors.add(2); return factors; } } Refactor – improve naming
publicclassPrimeFactorsTest { // ... @Test publicvoidfactorsOfTwo() { assertEquals(Arrays.asList(2), getPrimeFactors(2)); } @Test publicvoidfactorsOfThree() { assertEquals(Arrays.asList(3), getPrimeFactors(3)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); if (number == 2) factors.add(2); if (number == 3) factors.add(3); return factors; } } Simplest next case + code
publicclassPrimeFactorsTest { // ... @Test publicvoidfactorsOfTwo() { assertEquals(Arrays.asList(2), getPrimeFactors(2)); } @Test publicvoidfactorsOfThree() { assertEquals(Arrays.asList(3), getPrimeFactors(3)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); if (number > 1) factors.add(number); return factors; } } Refactor away duplication
publicclassPrimeFactorsTest { // ... @Test publicvoidfactorsOfFour() { assertEquals(Arrays.asList(2,2), getPrimeFactors(4)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); if (number == 4) { factors.add(2); number /= 2; } if (number > 1) factors.add(number); return factors; } } Next case Special case result
publicclassPrimeFactorsTest { // ... @Test publicvoidfactorsOfSix() { assertEquals(Arrays.asList(2,3), getPrimeFactors(6)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); if (number == 6) { factors.add(2); number /= 2; } if (number == 4) { factors.add(2); number /= 2; } if (number > 1) factors.add(number); return factors; } } Next case – duplicate special case
publicclassPrimeFactorsTest { // ... @Test publicvoidfactorsOfSix() { assertEquals(Arrays.asList(2,3), getPrimeFactors(6)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); int factor = 2; if (number % factor == 0) { factors.add(factor); number /= factor; } if (number > 1) factors.add(number); return factors; } } Refactor away duplication Important design step!
publicclassPrimeFactorsTest { // ... @Test publicvoidfactorsOfEight() { assertEquals(Arrays.asList(2,2,2), getPrimeFactors(8)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); int factor = 2; if (number % factor == 0) { factors.add(factor); number /= factor; } if (number > 1) factors.add(number); return factors; } } Next test – fails
publicclassPrimeFactorsTest { // ... @Test publicvoidfactorsOfEight() { assertEquals(Arrays.asList(2,2,2), getPrimeFactors(8)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); int factor = 2; while (number % factor == 0) { factors.add(factor); number /= factor; } if (number > 1) factors.add(number); return factors; } } Simplest thing to make it work!
publicclassPrimeFactorsTest { // ... @Test publicvoidfactorsOfNine() { assertEquals(Arrays.asList(3,3), getPrimeFactors(9)); } private List<Integer> getPrimeFactors(int number) { List<Integer> factors = newArrayList<>(); for (int factor = 2; factor<number; factor++) { while (number % factor == 0) { factors.add(factor); number /= factor; } } if (number > 1) factors.add(number); return factors; } } Next test and code
Think of next tests Write a test Make it pass as simply as possible Refactor: Add names, remove duplication
Think of next tests Write a test Make it pass as simply as possible Refactor: Add names, remove duplication
Think of next tests Write a test Make it pass as simply as possible Refactor: Add names, remove duplication
Think of next tests Write a test Make it pass as simply as possible Refactor: Add names, remove duplication
This is where the design comes: Refactor: Add names, remove duplication
You think too much! (about the wrong things)
describe("Modbus data server", function() { it("should read saved data from correct register", function(done) { var measurements = [ sampleMeasurement({ cassette: 1, transponder: 1, thickness: 50.1 }), sampleMeasurement({ cassette: 1, transponder: 2, thickness: 50.2 }), ]; updateData(measurements, function() { readRegisters(cassette_start_pos(1), 4, function(err, data) { data.readFloatLE(0).should.be.approximately(50.1, 0.0001); data.readFloatLE(4).should.be.approximately(50.2, 0.0001); done(); }); }); }); });
describe("Modbus data server", function() { it("should read saved data from correct register", function(done) { var measurements = [ sampleMeasurement({ cassette: 1, transponder: 1, thickness: 50.1 }), sampleMeasurement({ cassette: 1, transponder: 2, thickness: 50.2 }), ]; updateData(measurements, function() { readRegisters(cassette_start_pos(1), 4, function(err, data) { data.readFloatLE(0).should.be.approximately(50.1, 0.0001); data.readFloatLE(4).should.be.approximately(50.2, 0.0001); done(); }); }); }); }); Given
describe("Modbus data server", function() { it("should read saved data from correct register", function(done) { var measurements = [ sampleMeasurement({ cassette: 1, transponder: 1, thickness: 50.1 }), sampleMeasurement({ cassette: 1, transponder: 2, thickness: 50.2 }), ]; updateData(measurements, function() { readRegisters(cassette_start_pos(1), 4, function(err, data) { data.readFloatLE(0).should.be.approximately(50.1, 0.0001); data.readFloatLE(4).should.be.approximately(50.2, 0.0001); done(); }); }); }); }); When
describe("Modbus data server", function() { it("should read saved data from correct register", function(done) { var measurements = [ sampleMeasurement({ cassette: 1, transponder: 1, thickness: 50.1 }), sampleMeasurement({ cassette: 1, transponder: 2, thickness: 50.2 }), ]; updateData(measurements, function() { readRegisters(cassette_start_pos(1), 4, function(err, data) { data.readFloatLE(0).should.be.approximately(50.1, 0.0001); data.readFloatLE(4).should.be.approximately(50.2, 0.0001); done(); }); }); }); }); Then
var registers = newbuffer.Buffer(10000); varcassette_start_register = function(cassette_id) { return0; }; varreadRegisters = function(start_register, register_count, callback) { var message = registers.slice(start_register*2, start_register*2 + register_count*2); callback(null, message); }; varupdateData = function(objects, callback) { for (vari=0; i<objects.length; i++) { var measurement = objects[i]; vartransducer_pos = 2*cassette_start_register(measurement.cassette_id) + 4*(measurement.transponder_id - 1); registers.writeFloatLE(measurement.thickness, transducer_pos); } callback(); };
varSequelize = require("sequelize"); varsequelize = newSequelize('modbusfun', 'modbus', 'secret', { dialect: 'mysql', port: 3306, logging: false }); var Measurement = require('../lib/modbus-fun/measurement') .init(sequelize); varupdateRegisters = function(callback) { Measurement.findMeasurements(function(err, objects) { for (vari=0; i<objects.length; i++) { var measurement = objects[i]; vartransducer_pos = 2*cassette_start_register(measurement.cassette_id) + 4*(measurement.transponder_id - 1); registers.writeFloatLE(measurement.thickness, transducer_pos); } callback(); }); };
varmyRegisters = Registers.createRegisters(Measurement); var server = myRegisters.createSocket(); varmodbusClient; varreadRegisters = function(start, count, callback) { modbusClient.readRegisters(start, count, function(err, data) { callback(err, data); }); };
varreadRegisters = function(offset, register_count, callback) { callbacks[++transactionId] = function(response_message) { callback(0, response_message.slice(10, 10 + register_count*2)); }; var fc = 0x04; var message = newbuffer.Buffer(12); message.writeUInt16LE(transactionId, 0); message.writeUInt16LE(protocolId, 2); message.writeUInt16LE(6, 4); message.writeUInt8(unitId, 6); message.writeUInt8(fc, 7); message.writeUInt16LE(offset, 8); message.writeUInt16LE(register_count, 10); client.write(message); };