Technique: Moving functions between objects (refactoring by M. Fowler)
- Tutorial
Home Code with a choke
Technique: Compilation of methods
In the sequel, refactoring technique according to the book Refactoring. Improving existing code Martin Fowler.
The syntax will be in C #, but the main thing is to understand the idea, and it can be used in any other programming language.
As a rule, all disputes / disagreements arising between the reviewer and comitt-err can be divided into the following:
I hope to continue further “Technique: Data Organization”.
Technique: Compilation of methods
In the sequel, refactoring technique according to the book Refactoring. Improving existing code Martin Fowler.
The syntax will be in C #, but the main thing is to understand the idea, and it can be used in any other programming language.
Moving functions between objects
- Moving a method ( create a new method with a similar body in the class that uses it more often ).
The method should be in a class that better reflects its subject area
Fromclass View { private Shop shop; private List
FilterUsers(List users, decimal koef) { List activeUsers = new List (); foreach(User user in users) { if(shop.IsActiveUser(user, koef)) { activeUsers.Add(user); } } return activeUsers; } } class Shop { public bool IsActiveUser(User user, decimal koef) { decimal rate = 1; if(koef > 1000) { rate = koef * 1.75; } return user.Boss || user.HasDiscount() && rate > 10; } } class User { private bool boss; public bool Boss { get { return boss; } } public bool HasDiscount() { // some condition } }
Toclass View { private Shop shop; private List
FilterUsers(List users, decimal koef) { List activeUsers = new List (); foreach(User user in users) { if(user.IsActiveUser(koef)) { activeUsers.Add(user); } } return activeUsers; } } class Shop { } class User { private bool boss; public bool Boss { get { return boss; } } public bool HasDiscount() { // some condition } public bool IsActiveUser(decimal koef) { decimal rate = 1; if(koef > 1000) { rate = koef * 1,75; } return Boss || HasDiscount() && rate > 10; } } - Moving a field ( create a new field in the target class ).
The field should be in a class that better reflects its subject area
Fromclass Shop { private decimal rate; private DiscountType discountType; public Shop(DiscountType discountType, decimal rate) { this.rate = rate; this.discountType = discountType; } public decimal Rate { get{ return rate; } set{ rate = value; } } public bool HasDiscount(DateTime from, DateTime to) { // some calculation of flag return discountType.IsValid(rate) && flag; } public decimal GetDiscount(decimal amount, bool fullSum) { // some calculation of koef return discountType.GetAmount(rate, fullSum) * koef; } } class DiscountType { public bool IsValid(decimal rate) { // some calculation using rate } public decimal GetAmount(decimal rate, bool fullSum) { // some calculation using rate } }
Toclass Shop { private DiscountType discountType; public Shop(DiscountType discountType) { this.discountType = discountType; } public bool HasDiscount(DateTime from, DateTime to) { // some calculation of flag return discountType.IsValid() && flag; } public decimal GetDiscount(decimal amount, bool fullSum) { // some calculation of koef return discountType.GetAmount() * koef; } } class DiscountType { private decimal rate; public DiscountType(decimal rate) { this.rate = rate; } public decimal Rate { get{ return rate; } set{ rate = value; } } public bool IsValid() { // some calculation using rate } public decimal GetAmount(bool fullSum) { // some calculation using rate } }
- Highlighting a class ( create a new class and move the fields and methods from the old class to the new one ).
The class must contain its own data model and methods for working with it, otherwise it turns into a God object.
Fromclass User { private string name; private string street; private string city; public string Name { get{ return name; } set{ name = value; } } public string Street { get{ return street; } set{ city = value; } } public string City { get{ return city; } set{ city = value; } } public string GetAddressInfo() { return string.Format("{0} \ {1}", city, street); } }
Toclass User { private string name; private Address address; public string Name { get{ return name; } set{ name = value; } } public string GetAddressInfo() { return address.GetFullAddress(); } } class Address { private string city; private string street; public string Street { get{ return street; } set{ city = value; } } public string City { get{ return city; } set{ city = value; } } public string GetFullAddress() { return string.Format("Adress: {0} \ {1}", city, street); } }
- Inlining a class ( move all functions to another class and delete the original one ).
A class has too few functions.
Fromclass User { private string name; private Address address; public string Name { get{ return name; } set{ name = value; } } public Address GetAddress() { return address } } class Address { private string areaCode; private string country; public string AreaCode { get{ return areaCode; } set{ areaCode = value; } } public string Country { get{ return country; } set{ country = value; } } }
Toclass User { private string name; private string areaCode; private string country; public string AreaCode { get{ return areaCode; } set{ areaCode = value; } } public string Country { get{ return country; } set{ country = value; } } public string Name { get{ return name; } set{ name = value; } } }
- Hiding delegation ( create methods that hide delegation ).
Encapsulation, some parts of the system.
Fromclass View { private void Init() { // some code User manager = currentUser.Office.Manager; } } class User { private Department department; public Department Office { get{ return department; } set{ department = value; } } } class Department { private User manager; public Department(User manager) { this.manager = manager; } public User Manager { get{ return manager; } } }
Toclass View { private void Init() { // some code User manager = currentUser.Manager; } } class User { private Department department; public User Manager { get{ return department.Manager; } } } class Department { private User manager; public Department(User manager) { this.manager = manager; } public User Manager { get{ return manager; } } }
- Removing an intermediary ( force the client to contact the delegate directly ).
The class is too busy with simple delegation.
Fromclass View { private void Init() { // some code User manager = currentUser.Manager; decimal rate = currentUser.Rate; } } class User { private Department department; private UserType userType; public User Manager { get{ return department.Manager; } } public decimal Rate { get{ return userType.Rate; } } } class Department { private User manager; public User Manager { get{ return manager; } } } class UserType { private decimal rate; public decimal Rate { get{ return rate; } } }
Toclass View { private void Init() { // some code User manager = currentUser.Office.Manager; decimal rate = currentUser.Type.Rate; } } class User { private Department department; private UserType userType; public Department Office { get{ return department; } } public UserType Type { get{ return userType; } } } class Department { private User manager; public User Manager { get{ return manager; } } } class UserType { private decimal rate; public decimal Rate { get{ return rate; } } }
- Introduction of an external method ( at the client, add a method to which the server class is passed as the 1st parameter ).
It is necessary to introduce a method, but there is no possibility of modifying the class.
Fromclass View { private string GetUserName() { // some code return "\"" + userName + "\""; } private string GetCompanyName() { // some code return "\"" + companyName + "\""; } }
Toclass View { private string GetUserName() { // some code return userName.InDoubleQuote(); } private string GetCompanyName() { // some code return companyName.InDoubleQuote(); } } static class StringExtension { public static string InDoubleQuote(this string str) { return "\"" + str + "\""; } }
- Introduction of local extension ( create a wrapper for the class (or using inheritance) and add the necessary methods ).
Methods must be introduced, but there is no way to modify the class.
Fromsealed class Account { public decimal CalculateSum() { // calculation summ return summ; } }
Toclass ImportantAccount { private Account account; public decimal CalculateMaxSum() { // calculation rate return account.CalculateSum() * rate; } }
Conflicts of Interest (as part of Review).
As a rule, all disputes / disagreements arising between the reviewer and comitt-err can be divided into the following:
- Colleagues discuss the choice of a suitable solution, as disagreement arose over implementation. Everything within the framework of correct communication, they understand that they are doing a common thing and you just need to choose a solution (10%).
- Colleagues need to chat: Friday, a hard working week or just not enough communication. There is such a category of people who just need to crack. In principle, this is not without common sense, because communication and discussion is great (10%).
- Disputes that lead to new rules. For example, when the var keyword appeared in .NET, one of the colleagues began to use it everywhere, someone the other way around. Therefore, during the review, disputes arose, which is better. As a result, after some discussions, a decision was made (10%).
// если операнд с правой стороны, однозначно трактует возвращаемый тип, то можно использовать var var users = new List
(); var departments = user.Departmens.Cast (); // плохо var users = departments.Select(d => d.User).Where(u => !u.Deleted); var departments = service.GetNotDeletedDepartments(); - A heated debate over whose decision is better. And if programmers are at the same level, passions can flare up. Moreover, all these disputes, only a waste of time and a desire to prove themselves cool (70%). Here I want to give advice, if you got involved in such a dispute, never go over not an individual. And the phrase “Your decision does not fit” should be replaced with “The decision is not entirely correct and explain your point of view” . Those. the emphasis should be on discussing the decision, not the skills of a colleague.
Funny cases:
1) Review-er - “it is necessary to correct the code”
Commiter - “you do not like you and rule” / “you found you and correct”
2) Or how correctlyvoid Method() { ... if(flag) koef = GetMaxRate(); ... }
orvoid Method() { ... if(flag) { koef = GetMaxRate(); } ... }
I hope to continue further “Technique: Data Organization”.