Hot answers tagged

53

You're looking for tests that check for regressions. i.e. breaking some existing behaviour. I would start by identifying at what level that behaviour will remain the same, and that the interface driving that behaviour will remain the same, and start putting in tests at that point. You now have some tests that will assert that whatever you do below this ...


37

The recommended practice is to start with writing "pin-down tests" that test the current behaviour of the code, possibly including bugs, but without requiring you to descend into the madness of discerning whether a given behaviour that violates requirements documents is a bug, workaround for something you're not aware of, or represents an undocumented change ...


26

Removing a public method is not "refactoring" -- refactoring is changing the implementation while continuing to pass existing tests. However, removing an unneeded method is a perfectly reasonable design change. TDD draws this out to some extent, because in reviewing the tests, you may observe that it's testing an unneeded method. The tests are driving your ...


16

Yes, of course. The easiest code to read is that which isn't there. That said, refactoring generally means improving the code without changing its behavior. If you think of something that improves the code, just do it. There's no need to fit it into some pigeon hole before you're allowed to do it.


13

Reusing code is not about resurrecting old unused and abandoned code and putting it to use. Reusing code is about making the same code fulfill multiple purposes (perhaps within the same project) by providing a useful abstraction that can be (re)applied in other contexts. Old abandoned code may be consulted, but should probably not be resurrected. It ...


13

I suggest - if you haven't already - reading both Working Effectively With Legacy Code as well as Refactoring - Improving the Design of Existing Code. [..] The problem it appears to me is that when I do refactor then those tests will break as I'm changing where certain logic is done and the tests will be written with the previous structure in mind ...


9

In fact f() replaces all calls to b() with the exception of the unit tests that defined / described b() IMHO the typical TDD cycle will look like this: write failing tests for f() (probably based on the tests for b()): tests go red implement f() -> tests become green refactor: -> remove b() and all tests for b() For the last step, you might consider ...


5

Don't write strict unit tests where you mock all the dependencies. Some people will tell you these aren't real unit tests. Ignore them. These tests are useful, and that's what matters. Let's look at your example: public class MyDocumentService { ... public List<Document> findAllDocuments() { DataResultSet rs = ...


4

Yes, it is. The best, most bug-free, most readable code is the code that doesn't exist. Strive to write as much non-code as possible while fulfilling your requirements.


4

If you want your refactorings to "live indepently" from your current feature branch, do not make the changes there. Instead, do the refactoring on the main development branch (or a "refactoring branch", if it is common in your team not to apply changes directly to the dev branch). So anyone of your team (including you) can merge the changes into the active ...


3

As you say, if you change the behaviour then it is a transformation and not a refactor. At what level you change the behaviour is what makes the difference. If there are no formal tests at the highest level then try and find a set of requirements that clients (calling code or humans) that need to stay the same after your redesign for your code to be ...


3

tl;dr Don't write unit tests. Write tests at a more appropriate level. Given your working definition of refactoring: you don't change what your software does, you change how it does it there is very wide spectrum. On one end is a self-contained change to particular method, perhaps using a more efficient algorithm. On the other end is porting to ...


3

You should only 'fix' code in a feature branch if you are changing that piece of code anyway as part of the feature. Eg. I am working on the 'print rabbits' feature and I find the printer code Public class Printer(string type) { If(type=="bunnies") { //print a bunny } ..... } I change it to: Public class Printer(string type) { ...


2

One thing to always try to remember is that we're now using CODE REPOSITORIES with VERSION CONTROL. That deleted code isn't actually gone...it's still there somewhere in a previous iteration. So blow it away! Be liberal with the delete key, because you can always go back and retrieve that precious elegant method that you thought might be useful ...


2

It's desirable to remove b() once it's no longer used, for the same reason that it's desirable not to add un-used functions in the first place. Whether you call it "readability" or something else, all else being equal it's an improvement to the code that it doesn't contain anything it has no use for. For the sake of having at least one specific measure by ...


2

I will share with you the point of view that an architect of my company shared with me. If the cost (in time) of to review code, to understand what it does, to test it, to fix it (in your case to complet it) and to integrate is more expensive than to do it from scratch, then start from scratch As Erik pointed you should not trust in code that (as you ...


2

Don't waste time writing tests that hook in at points where you can anticipate that the interface is going to change in a non-trivial way. This is often a sign that you are trying to unit-test classes that are 'collaborative' in nature - whose value isn't in what they do themselves, but in how they interact with a number of closely-related classes to produce ...


2

Here my approach. It has a cost in terms of time because it's a refactor-test in 4 phases. What I'm going to expose may suite better in components with more complexity than the one exposed in the question's example. Anyways the strategy is valid for any component candidate to be normalized by an interface (DAO, Services, Controllers,...). 1. The interface ...


1

Your first task is to try to come up with the "ideal method signature" for your tests. Strive to make it a pure function. This should be independent of the code that is actually under test; it is a small adapter layer. Write your code to this adapter layer. Now when you refactor your code, you only need to change the adapter layer. Here is a simple example: ...


1

You don't reuse code whenever possible. You reuse code when reusing the code is more beneficial to you than writing new code. It looks like the most efficient way to handle this is to design everything properly, especially if you think that you need code doing several very similar but not identical things, and then when you implement the code, scavenge ...


1

Given: the application may be split up into different parts (business fields) most likely. and the application might be split apart into single services in the next few years I would be reluctant to do anything based on the above. Who knows what will happen in the near future, and I'd be wary of basing any design on what is predicted to ...


1

The purpose of branch types are to provide intention for handling them. If you're following a GitFlow style of branching, then you likely have types like feature, hotfix, release, etc.. In the case of a feature branch, its intention is to encapsulate a merge into another branch (i.e. develop) that shows the developer responsible for merging, what this ...


1

The problem seems to me that you are working excessively long on branches. Cost of conflicts grows exponentially with the length everyone stays on a branch, so with very long conflicts you have little chance of doing any refactoring.



Only top voted, non community-wiki answers of a minimum length are eligible