리팩토링을 망설이지 말자

Posted by epicdev Archive : 2012. 3. 29. 15:16

리팩토링을 망설이지 말자. 리팩토링은 시작이 반이다.

때때론 "리팩토링 하자"라고 마음 먹기가 리팩토링 하는 것보다 힘들다.

일단 리팩토링을 시작하면 생각보다 금방 끝나는 경우가 많다.

(코딩하면서 리팩토링을 꾸준히 했을경우. 켄트 백의 말을 빌리자면 코드를 조금씩 조금씩 성장시켰을 경우)


작은 리팩토링의 반복이 가장 바람직한 리팩토링인 것 같다.

마구 코딩만 하다가 "나중에 시간나면 리팩토링 해야지"하고 생각하다가

나중에 한꺼번에 리팩토링 하려면 그 때는 리팩토링이 거의 "불가능" 해지는 경우가 많다.

  

Encapsulate Downcast (다운캐스트의 캡슐화)

Posted by epicdev Archive : 2012. 1. 25. 00:26
문제: 메소드의 리턴 값을 호출하는 쪽에서 다운캐스트하지 않으면 안 된다.

해법: 다운캐스트를 메소드 내부에서 하자.


  

Encapsulate Collection (컬렉션의 캡슐화)

Posted by epicdev Archive : 2012. 1. 25. 00:22
문제: 변경가능한 컬렉션을 반환하는 getter 메소드가 있기 때문에 외부에서 컬렉션을 직접 조작한다.


리팩토링입문
카테고리 컴퓨터/IT > 프로그래밍/언어
지은이 히로시 유키 (한빛미디어, 2007년)
상세보기


  

리팩토링입문
카테고리 컴퓨터/IT > 프로그래밍/언어
지은이 히로시 유키 (한빛미디어, 2007년)
상세보기

  

Chain Constructors (연쇄 생성자)

Posted by epicdev Archive : 2012. 1. 24. 23:04

디폴트 생성자에서 rand = new Random(DEFAULT_SEED);를 호출하지 않고
long seed를 인자로 받는 다른 생성자를 호출하는 방식을 통해 중복되는 코드를 피하고 있다. 
  

A Taxonomy for "Bad Code Smells"

Posted by epicdev Archive : 2011. 11. 15. 15:18
출처: http://www.soberit.hut.fi/mmantyla/BadCodeSmellsTaxonomy.htm

A Taxonomy for "Bad Code Smells"

Citation

If you wish to cite this taxonomy please use the following article

Mäntylä, M. V. and Lassenius, C. "Subjective Evaluation of Software Evolvability Using Code Smells: An Empirical Study". Journal of Empirical Software Engineering, vol. 11, no. 3, 2006, pp. 395-431. (pdf)

Taxonomy

The reason for creating the taxonomy is to provide better understanding of the smells and to recognize the relationships between smells. I feel that with a long flat list of the code smells it is easy to lose the overall picture (at least that happened to me when I first studied the list of code smells in (Fowler&Beck 2000)). Thus, here is a taxonomy of five groups.

Group name

Smells in group

Discussion

The Bloaters -Long Method
-Large Class
-Primitive Obsession
-Long Parameter List
-DataClumps
Bloater smells represents something that has grown so large that it cannot be effectively handled.

It seems likely that these smells grow a little bit at a time. Hopefully nobody designs, e.g., Long Methods.

Primitive Obsession is actually more of a symptom that causes bloats than a bloat itself. The same holds for Data ClumpsWhen a Primitive Obsession exists, there are no small classes for small entities (e.g. phone numbers). Thus, the functionality is added to some other class, which increases the class and method size in the software. With Data Clumps there exists a set of primitives that always appear together (e.g. 3 integers for RGB colors). Since these data items are not encapsulated in a class this increases the sizes of methods and classes. 

The Object-Orientation Abusers -Switch Statements
-Temporary Field
-Refused Bequest
-Alternative Classes with Different Interfaces
The common denominator for the smells in the Object-Orientation Abuser category is that they represent cases where the solution does not fully exploit the possibilities of object-oriented design.

For example, a Switch Statement might be considered acceptable or even good design in procedural programming, but is something that should be avoided in object-oriented programming. The situation where switch statements or type codes are needed should be handled by creating subclasses. Parallel Inheritance Hierarchies and Refused Bequest smells lack proper inheritance design, which is one of the key elements in object-oriented programming. The Alternative Classes with Different Interfaces smell lacks a common interface for closely related classes, so it can also be considered a certain type of inheritance misuse. The Temporary Field smell means a case in which a variable is in the class scope, when it should be in method scope. This violates the information hiding principle.

The Change Preventers -Divergent Change
-Shotgun Surgery 
-Parallel Inheritance Hierarchies
Change Preventers are smells is that hinder changing or further developing the software

These smells violate the rule suggested by Fowler and Beck which says that classes and possible changes should have a one-to-one relationship. For example, changes to the database only affect one class, while changes to calculation formulas only affect the other class.

The Divergent Change smell means that we have a single class that needs to be modified by many different types of changes. With the Shotgun Surgery smell the situation is the opposite, we need to modify many classes when making a single change to a system (change several classes when changing database from one vendor to another)

Parallel Inheritance Hierarchies, which means a duplicated class hierarchy, was originally placed in OO-abusers. One could also place it inside of The Dispensables since there is redundant logic that should be replaced.

The Dispensables -Lazy class
-Data class 
-Duplicate Code
-Dead Code, 
-Speculative Generality
The common thing for the Dispensable smells is that they all represent something unnecessary that should be removed from the source code.

This group contains two types of smells (dispensable classes and dispensable code), but since they violate the same principle, we will look at them together. If a class is not doing enough it needs to be removed or its responsibility needs to be increased. This is the case with the Lazy class and the Data class smells. Code that is not used or is redundant needs to be removed. This is the case with Duplicate Code, Speculative Generality and Dead Code smells. 

The Couplers -Feature Envy
-Inappropriate Intimacy
-Message Chains 
-Middle Man
This group has four coupling-related smells.

One design principle that has been around for decades is low coupling (Stevens et al. 1974) . This group has 3 smells that represent high coupling. Middle Man smell on the other hand represent a problem that might be created when trying to avoid high coupling with constant delegation. Middle Man is a class that is doing too much simple delegation instead of really contributing to the application.

The Feature Envy smell means a case where one method is too interested in other classes, and the Inappropriate Intimacy smell means that two classes are coupled tightly to each other. Message Chains is a smell where class A needs data from class D. To access this data, class A needs to retrieve object C from object B (A and B have a direct reference). When class A gets object C it then asks C to get object D. When class A finally has a reference to class D, A asks D for the data it needs. The problem here is that A becomes unnecessarily coupled to classes B, C, and D, when it only needs some piece of data from class D. The following example illustrates the message chain smell: A.getB().getC().getD().getTheNeededData()

Of course, I could make an argument that these smells should belong to the Object-Orientation abusers group, but since they all focus strictly on coupling, I think it makes the taxonomy more understandable if they are introduced in a group of their own.

FAQ

In what way is this taxonomy better than the one provided in the Refactoring Workbook (Wake 2003)? 
It is shorter ;-). Seriously, this is left as an exercise for the reader. If you like mine better - great. If you like their better - fine.

Hey, I could argue that all smells should be placed inside a certain group (Change Preventers or OO abusers) since they all prevent change / abuse OO principles. 
This is true. However, then we would not receive any increased understanding from the classification.

I want to learn more about refactoring and code smells
Try (Fowler 2000) or www.refactoring.com

I need to find a really good agile process framework to help our process improvement!
Great, try the SEMS-approach

References

M. Fowler, Refactoring: Improving the Design of Existing Code, Canada: Addison-Wesley, 2000.

M. Fowler and K. Beck, "Bad Smells in Code," in Refactoring: Improving the Design of Existing Code, Addison-Wesley, 2000, pp. 75-88.

W. Stevens, G. Myers and L. Constantine, "Structured Design," IBM Syst J, vol. 13, no. 2, pp. 115-139, 1974.

W.C. Wake, Refactoring Workbook, Addison Wesley, 2003.


comments / feedback: Mika Mäntylä

  

'Archive' 카테고리의 다른 글

In Praise Of Small Code  (0) 2011.11.15
Hollywood Principle  (0) 2011.11.15
Code Smell  (0) 2011.11.15
테스트가 코드를 향상시키도록 하라  (0) 2011.11.15
Agile 원칙 12가지  (0) 2011.11.14
  

Code Smell

Posted by epicdev Archive : 2011. 11. 15. 15:17
출처: http://en.wikipedia.org/wiki/Code_smell

In computer programmingcode smell is any symptom in the source code of a program that possibly indicates a deeper problem.

Often the deeper problem hinted by a code smell can be uncovered when the code is subjected to a short feedback cycle where it is refactored in small, controlled steps, and the resulting design is examined to see if there are any further code smells that indicate the need of more refactoring. From the point of view of a programmer charged with performing refactoring, code smells are heuristics to indicate when to refactor, and what specific refactoring techniques to use. Thus, a code smell is a driver for refactoring.

The term appears to have been coined by Kent Beck on WardsWiki in the late 1990s. Usage of the term increased after it was featured in Refactoring. Improving the Design of Existing Code.[1] Code Smell is also a term used by agile programmers. [2]

Determining what is and is not a code smell is often a subjective judgment, and will often vary by language, developer and development methodology. There are tools, such as Checkstyle, PMD and FindBugs for Java, to automatically check for certain kinds of code smells. 

Common Code Smells
  • Duplicated code: identical or very similar code exists in more than one location.
  • Long method: a method, function, or procedure that has grown too large.
  • Large class: a class that has grown too large. See God object.
  • Too many parameters: a long list of parameters in a procedure or function make readability and code quality worse.
  • Feature envy: a class that uses methods of another class excessively.
  • Inappropriate intimacy: a class that has dependencies on implementation details of another class.
  • Refused bequest: a class that overrides a method of a base class in such a way that the contract of the base class is not honored by the derived class. See Liskov substitution principle.
  • Lazy class / Freeloader: a class that does too little.
  • Contrived complexity: forced usage of overly complicated design patterns where simpler design would suffice.
  • Excessively long identifiers: in particular, the use of naming conventions to provide disambiguation that should be implicit in the software architecture.
  • Excessively short identifiers: the name of a variable should reflect its function unless it's obvious.
  • Excessive use of literals: these should be coded as named constants, to improve readability and to avoid programming errors. Additionally, literals can and should be externalized into resource files/scripts where possible, to facilitate localization of software if it is intended to be deployed in different regions. 

출처:  http://c2.com/cgi/wiki?CodeSmell

 code smell is a hint that something has gone wrong somewhere in your code. Use the smell to track down the problem. KentBeck (with inspiration from the nose of MassimoArnoldi) seems to have coined the phrase in the "OnceAndOnlyOnce" page, where he also said that code "wants to be simple". Bad Smells in Code was an essay by KentBeck and MartinFowler, published as Chapter 3 of RefactoringImprovingTheDesignOfExistingCode.

Highly experienced and knowledgeable developers have a "feel" for good design. Having reached a state of "UnconsciousCompetence," where they routinely practice good design without thinking about it too much, they find that they can look at a design or the code and immediately get a "feel" for its quality, without getting bogged down in extensive "logically detailed arguments".

Note that a CodeSmell is a hint that something might be wrong, not a certainty. A perfectly good idiom may be considered a CodeSmell because it's often misused, or because there's a simpler alternative that works in most cases. Calling something a CodeSmell is not an attack; it's simply a sign that a closer look is warranted.

[Copied from GotoConsideredHarmful:]

There are two major approaches to programming: [is there a page that covers this? there should be]
  • Pragmatic: CodeSmells should be considered on a case by case basis
  • Purist: all CodeSmells should be avoided, no exceptions
...And therefore a CodeSmell is a hint of possible bad practice to a pragmatist, but a sure sign of bad practice to a purist.

If I recall correctly, this is part of the reason why (was it KentBeck or MartinFowler) chose this name for the term. They wanted to emphasize the pragmatic view of CodeSmells by the connotation of the word smell. If something smells, it definitely needs to be checked out, but it may not actually need fixing or might have to just be tolerated.

See also

Here are some Code Smells that tell you something might be wrong.

Too much code, time to take something off the stove before it boils over:

Not enough code, better put the half-baked code back in the oven a while:

  • Classes with too few instance variables
  • Classes with too little code - See: OneResponsibilityRule
  • Methods with no messages to self - See: UtilityMethod
  • EmptyCatchClauses
  • Explicitly setting variables to null. Can indicate that either
    • there are references to things that this code has no business referencing, or
    • the structure is so complex that the programmer doesn't really understand it and feels the need to do this to be safe.
    • Is this too language specific? For example in Perl and other agile/scripting languages setting a variable to null can be the equivalent of destroying an object. -- MarkAufflick

Not actually the code:

Problems with the way the code is changing:

  • Sporadic ChangeVelocity - Different rates of change in the same object.
  • ShotgunSurgery - The same rate of change in different, disconnected objects.

Other code problems:

  • ContrivedInterfaces - A smell of PrematureGeneralization.
  • AsymmetricalCode/Imbalance
  • ArrowAntiPattern (nested if statements), Complex Conditionals
  • LawOfDemeter Violations - Messages to the results of messages.
  • Dependency cycles among packages or classes within a package.
  • Concrete classes that depend on other concrete classes
  • Methods requiring SpecialFormatting to be readable
  • BackPedalling (loss of context)
  • Long method names. Seriously: If you follow good naming standards, long method names are often an indication that the method is in the wrong class. For example, createWhateverFromWhateverOtherClass(OtherClass? creator) vs creator.createWhatever(). See UsingGoodNamingToDetectBadCode.
  • VagueIdentifiers
  • Procedural code masquerading as objects. See DontNameClassesObjectManagerHandlerOrData
  • Embedded code strings. Large chunks of SQL, HTML or XML (for example) are not best read, edited or tested in the code. They start there because its simpler, but end up making both languages unreadable as they get more complex, and require developers to know both languages.
  • PassingNullsToConstructors - use a FactoryMethod
  • TooManyParametersLongParameterList
  • VariableNameSameAsType
  • WhileNotDoneLoops
  • False unification of procedures. A procedure, function, or method has a boolean that provides a variation on its behavior, but the two variations in fact have completely different semantics. It would be better to refactor the common code into another method and split the remainder into two separate methods, removing the boolean.
  • False unification of interfaces. An interface has two implementors. One implementor implements half of the interface and throws UnsupportedOperationException for the other half. The second implementor implements the other half and throws UnsupportedOperationException for the first half. It would be better to split the interface into two. Similar to RefusedBequest?.
  • Hidden coupling. Code depends on completely non-obvious characteristics, such as relying on reference equality instead of value equality.
  • Hardwired policy. Instead of policy being wrapped around a mechanism, the policy is wired directly into the mechanism. Often, implementation of the policy requires additional information not available, so it has to be fetched from outside, creating an implicit external dependency. Instead of a simple, self-contained mechanism, you now have a fragile, context-sensitive mechanism.

From BadSmellsInCode?: The list of canonical smells found in RefactoringImprovingTheDesignOfExistingCode can be found at: http://jexp.de/papers/refactoring/refactoring/node26.html (we're slowly making sure they're all here).

-- updated broken link, Michael Hunger

Some Principles for judging whether code smells bad or good are defined in PrinciplesOfObjectOrientedDesign.

See Also:

[This list needs refactoring to above:] CodeSmells referenced in:

'Archive' 카테고리의 다른 글

Hollywood Principle  (0) 2011.11.15
A Taxonomy for "Bad Code Smells"  (0) 2011.11.15
테스트가 코드를 향상시키도록 하라  (0) 2011.11.15
Agile 원칙 12가지  (0) 2011.11.14
다섯가지의 소프트웨어 테스트  (0) 2011.11.14
  

리팩토링

Posted by epicdev Archive : 2011. 10. 4. 16:53
리팩토링이 필요한 코드를 일종의 '종양'이라고 생각하자. 종양을 제거하려면 수술이 필요하다. 지금 바로 수술해서 아직 종양이 작을 때 제거 할 수도 있다. 하지만 종양이 자라고 다른 곳으로 전이 할 때까지 놓아 둘 수도 있다. 하지만 그 때가 되면 제거하는 데 드는 비용도 더 커질 뿐더러 위험도 훨씬 커진다. 시간을 더 끌면, 환자는 생명을 잃을지도 모른다.

<실용주의 프로그래머 팁>
일찍 리팩토링하고, 자주 리팩토링하라

리팩토링해야 할 것들의 명단을 만들고 유지하라. 어떤 것을 지금 당장 리팩토링하기 힘들다면, 일정에 그것을 리팩토링 할 시간을 확실히 포함시켜 두도록 한다. 그 코드를 사용하는 사람들이 코드가 조만간 리팩토링 될 것이라는 사실과 그 사실이 그들의 코드에 어떤 영향을 주게 될지 인지하도록 만들어야 한다.

리팩토링은 천천히, 신중하게, 조심스럽게 진행해야 하는 작업이다. 마틴 파울러는 손해보다 이득이 큰 방향으로 리팩토링을 하기 위한 다음 몇가지 간단한 조언을 제공한다.

1. 리팩토링과 새로운 기능 추가를 동시에 하지 말라.
2. 리팩토링을 시작하기 전 든든한 테스트 집합이 있는지 먼저 확인한다. 할 수 있는 한 자주 테스트들을 돌려본다. 이렇게 하면 여러분의 변경 때문에 무엇이 망가졌을 경우 재빨리 그 사실을 알 수 있다.
3. 단계를 작게 나누어서 신중하게 작업한다. 필드를 한 클래스에서 다른 클래스로 옮기기, 비슷한 메소드를 합쳐서 수퍼클래스로 옮기기. 리팩토링에서는 국지적인 변경들이 많이 모여서 커다란 규모의 변화를 낳는 일이 자주 발생한다. 단계를 작게 나누고, 한 단계가 끝날 때마다 테스트를 돌린다면, 기나긴 시간의 디버깅 작업을 피할 수 있다.

모듈에 큰 변화가 있다면, 즉 모듈의 인터페이스나 기능을 이전과 호환성을 유지 할 수 없을 정도로 변경하는 변화가 있다면, 일부러 빌드를 실패하도록 변화를 주는 기법도 유용하다. 리팩토링 대상 코드에 의존하는 옛날 코드들이 컴파일이 안 되게 만들어 버리는 것이다. 그러면 리팩토링 대상 코드에 어떤 코드들이 의존하는지 쉽게 찾아내서 지금 상황에 맞도록 고칠 수 있다.

그러므로 다음 번에 여러분이 마땅하다고 생각하는 수준에 못 미치는 코드를 보게 되면, 그 코드와 더불어 그 코드에 의존하는 모든 것도 함께 고치도록 한다. 고통을 관리하자. 지금 고통스럽더라도, 앞으로 더욱 고통스러워질 것 같으면 지금 고치는 편이 낫다.

실용주의프로그래머
카테고리 컴퓨터/IT > 프로그래밍/언어
지은이 앤드류 헌트 (인사이트, 2007년)
상세보기
  
다른 개발자들에게 API를 제공한다는 마음으로 개발하라
남이 봐도 쉬운 코드를 만들어라
'역사적'인 이유를 만들지 말아라
자신의 코드만 보지 말아라
기존의 코드와 통일성 있는 코드를 작성하라
커뮤니케이션 하라
항상 '1년 뒤에 이 소스를 본다면?' 이라고 생각하라 
리팩토링 하라 


패턴그리고객체지향적코딩의법칙
카테고리 컴퓨터/IT > 프로그래밍/언어
지은이 문우식 (한빛미디어, 2007년)
상세보기

  
 «이전 1  다음»