Archive for the ‘misc’ Category

デバッグするとバグが出る?! : DLLにまつわる問題

Monday, April 10th, 2006

※ このアーティクルはWindows/NT環境下でMicrosoft Visual C++ 6.0 SP3 による開発の最中に出くわした’厄介’なバグとその顛末です。

debug-modeに切り換えたとたんバグが消えてしまうという症状に悩まされるのはよくあることですが、逆にdebug-modeでしか発生しないバグという何とも奇妙な…

何が起こったのか

ここに掲載するアーティクルのため、小さなお試しコードを書いていました。XMLのチュートリアルXMLを用いた状態遷移で、IBM alphaWorksの XML4C(XML Parser for C++)を使ってXMLファイルをパースする試みの最中でした。

「こんなのどーってことないや」とタカをくくって鼻歌混じりにコードをこさえ、コンパイルも実行もいたって正常に完了しました。

僕は普段はもっぱらrelease-modeでコンパイル/実行を繰り返しながら開発を進めていきます。debug-modeは、たまに不可解なバグに遭遇するときにお世話になる程度なんです。

で、何の気なしにdebug-modeでコンパイルし、実行してみると突如として

Debug Assertion Failed!

なんてダイアログが現われたのです。release-modeでは何の問題もなかったというのに…

Visual Studioのデバッガは以下のコードがヘンだと主張していました:

  DOMString x("hello");
  char* tmp = x.transcode();
  cout << tmp << endl;
  delete[] tmp; // ここでassert

この最後の delete[] tmp;で"不正なヒープ領域を解放した!"と怒っているようです。

XML4Cが提供するクラスDOMStringはその名のとおりDOM(Document Object Model)内で使われているUNICODE文字列です。DOMStringのメソッドtranscode()は内包するUNICODE文字列をnativeなコード、すなわちShift_JISに変換し、その先頭ポインタを返してくれます。XML4Cのマニュアルには、

DOMString::transcode() で得られたポインタはきちんと解放するように

と書かれていたので、ぼくはその注意を忠実に守っただけですよ…

その原因は?

いくらrelease-modeでおっけーだったからといって、debug-modeでこんなシビアなassert吐くようでは納得いきません。コード生成オプションをさんざんいじくりまわしてダメ!と言われる組み合わせを模索し、ようやくその原因を突き止めました…

XML4CはDLLとリンクライブラリ、そしてヘッダファイルで構成されています。「もしや…」と思ってDLLが使っているDLLを調べたところ、MSVCRT.DLLが使われていたんです。

MSVCRT.DLLとは、Visual C++のruntime-libraryをDLL化したものです。runtime-libraryをDLL化し、それを呼び出すことにすれば、それぞれの実行モジュールにruntime-libraryがリンクされませんから、実行モジュール(ここではDLL)はそれだけコンパクトになります。

不具合の原因はここにありました。

XML4Cは変換結果を格納する領域をXML4C自身が確保します。このとき operator new が使われています。これは最終的には runtime-library が提供するヒープ領域確保関数が呼び出されることになります。つまりこれはMSVCRT.DLLにある確保関数です。

一方、XML4Cを利用するアプリケーションをdebug-modeで構築すると、マルチスレッド/rutime-DLL(オプション:-MDd)のとき、使われるライブラリはMSVCRTD.DLLです。

すると、DLLとアプリケーションとで、使われるruntime-libraryが異なることになります。

この状況においても通常はほとんど問題は生じません。が、僕が遭遇した状況では、(DLLが使っている)release版runtimeが確保した領域を(アプリケーションが使っている)debug版runtimeが解放することになります。

debug版runtimeのヒープ領域確保関数はデバッグのための小さな領域を余分に確保し、解放関数でそこを検証することでヒープ領域の不正なアクセスを検出します。

release版runtimeが確保したヒープ領域には、そのデバッグ用のエリアが存在しません。にもかかわらずdebug版の解放関数が検証を行ない、結果的に異常と判断されたのです。

どうやって解決したか

ほとんどの場合、DLLがどんなコンパイル・オプションで構築されたかなんて気にする必要はありません(し、実際気にしないでしょう)。その意味で今回の例は非常に特殊といえます。DLLのコンパイル・オプションまで調査するハメになったのは、DLLが確保した領域をアプリケーションが解放するという特殊な場合だからです。

このDLLを作ったのが僕ならば、確保した領域を解放する関数もついでに提供するでしょう。そうすれば領域の解放はDLL内で行われるため、領域確保時との整合性が保てます。

しかしながらXML4Cは天下のIBM製、ソースコードが公開されているとはいえ、勝手にいぢくるわけにもまいりません。使う側で何らかの手段を講じることにしました。

まず、DLLとアプリケーションとのコンパイルオプションを一致させること。XML4Cを使うときは:

  • マルチスレッド
  • ランタイムDLL
  • release-mode

でなくてはなりません(コンパイル・オプション -MD)。

最初の2つについてはコードの冒頭に以下のような’オマジナイ’を書いておくことで適合しないコンパイル・オプションを抑止しました:

/* マルチスレッド / runtime-DLL でないならコンパイル停止 */
#if !defined(_MT) || !defined(_DLL)
#  error sorry, multithread & runtime-DLL only.
#endif

まことに厳しいのが3番目の制約です。これはすなわちXML4Cを使うアプリケーションをdebug-mode(コンパイル・オプション-MDd)で構築できないことを意味します。

XML4CのAPIドキュメントを丹念に読み返し、幸いにもライブラリ側で領域を取るんじゃなくて、アプリケーション側で用意した領域に出力するUNICODE->native文字列の変換関数 XMLString::transcode() を見つけました。debug-modeではこれを使うようにコードの修正を行ないました。

#ifndef _DEBUG

  // release-mode
  std::string transocode(const DOMString& x)  {
    char* tmp = x.transcode();
    std::string ret(tmp);
    delete[] tmp;
    return ret;
  }

#else

  // debug-mode 変換結果を格納する領域はアプリケーション側で確保する
  std::string  transcode(const DOMString& x)  {
    const wchar_t* buf = x.rawBuffer(); // UNICODEバッファと
    unsigned       len = x.length();        // その長さ
    wchar_t*       wcs = new wchar_t[len+1]; // 領域確保
    char*          mbs = reinterpret_cast<char*>(wcs);
    *std::copy(buf, buf+len, wcs) = L'\0'; // UNICODEバッファからコピー
    XMLString::transcode(wcs, mbs, len*2+1); // nativeコードに変換

    std::string ret(mbs); // std::stringを生成
    delete[] wcs; // 領域解放
    return ret;
  }

#endif

  DOMString x("Hello");
  cout << transcode(x) << endl;

ま、こんなやりかたで急場をしのいだわけですが、このwork-aroundはできれば避けたいですね。だってrelase時とdebug時では異なるコードがコンパイルされるわけで、それでは何の為のdebugだかわかりませんからね。

教訓 : DLLが領域確保する関数を作ったら、その領域を解放する関数も公開すべし!

C++/C#/VB.NETによる リファクタリング – 最初の例

Thursday, April 6th, 2006

リファクタリング(Refactoring)’をご存知ですか? 「外部から見たときの振る舞いを保ちつつ、理解や修正が簡単になるように、ソフトウェアの内部構造を変化させること」です。

プログラムの改造や拡張に先立って適切なリファクタリングを行うことでその後の作業が楽になります。逆に’乱れた’状態のまま改造/拡張を繰り返すと次第にあちこちにほころびが目立ち始め、バグも出やすくなります。リファクタリングを行わずに改造/拡張を続けたソフトウェアは、さながら建て増しを繰り返して迷路と化した温泉旅館の様相を呈してきます。

このリファクタリングの様々なテクニックや定石を解説した本:

“リファクタリング – プログラム体質改善のテクニック”

マーチン・ファウラー 著

ピアソン・エデュケーション IDBN4-89471-228-8

は、変化に強く、単純で美しいコードを維持するための頼りになるガイドブックです。是非ご一読を。

この本は第一章:「リファクタリング-最初の例」から始まります。ビデオレンタルの料金計算を題材に、コード群にリファクタリングを施していく過程が詳しく解説されています。

「リファクタリング」では言語としてJavaが用いられています。C++/C#/VB.NETに翻訳するのはさほど苦労することはありませんが、やはり「C++/C#/VB.NET によるリファクタリング-最初の例」が欲しくなります。

また、この章ではリファクタリングによって最終的にどんなコードへと変貌を遂げたのか、その’生まれ変わったリスト’が示されていません。

そこで、本アーティクルでは’リファクタリング前’と’リファクタリング後’のそれぞれのコードを Java, C++, C#, および VB.NET で示します。あなたのコードを磨くサンプルとしてお役立てください。

リファクタリング前

リファクタリング後

C++/C#/VB.NETによる リファクタリング – リファクタリング前(Java)

Thursday, April 6th, 2006

Movie.java

// Movie.java

public class Movie {

  public static final int CHILDRENS = 2;
  public static final int REGULAR = 0;
  public static final int NEW_RELEASE = 1;

  private String _title;
  private int _priceCode;

  public Movie(String title, int priceCode) {
    _title = title;
    _priceCode = priceCode;
  }

  public int getPriceCode() {
    return _priceCode;
  }

  public void setPriceCode(int arg) {
    _priceCode = arg;
  }

  public String getTitle() {
    return _title;
  }

}

Rental.java

// Rental.java

public class Rental {

  private Movie _movie;
  private int _daysRented;

  public Rental(Movie movie, int daysRented) {
    _movie = movie;
    _daysRented = daysRented;
  }

  public int getDaysRented() {
    return _daysRented;
  }

  public Movie getMovie() {
    return _movie;
  }

}

Customer.java

// Customer.java
import java.util.*;

public class Customer {

  private String _name;
  private Vector _rentals = new Vector();

  public Customer(String name) {
    _name = name;
  }

  public void addRental(Rental arg) {
    _rentals.addElement(arg);
  }

  public String getName() {
    return _name;
  }

  public String statement() {
    double totalAmount = 0;
    int frequentRenterPoints = 0;
    Enumeration rentals = _rentals.elements();
    String result = "Rental Record for " + getName() + "\n";
    while ( rentals.hasMoreElements() ) {
      double thisAmount = 0;
      Rental each = (Rental)rentals.nextElement();
      // 一行ごとに金額を計算
      switch ( each.getMovie().getPriceCode() ) {
        case Movie.REGULAR :
          thisAmount += 2;
          if ( each.getDaysRented() > 2 )
            thisAmount += (each.getDaysRented() - 2) * 1.5;
          break;
        case Movie.NEW_RELEASE :
          thisAmount += each.getDaysRented() * 3;
          break;
        case Movie.CHILDRENS :
          thisAmount += 1.5;
          if ( each.getDaysRented() > 3 )
            thisAmount += (each.getDaysRented() - 3) * 1.5;
          break;
      }
      // レンタルポイントを加算
      frequentRenterPoints++;
      // 新作を二日以上借りた場合はボーナスポイント
      if ( (each.getMovie().getPriceCode() == Movie.NEW_RELEASE) &&
           each.getDaysRented() > 1 )
        frequentRenterPoints++;
      // この貸し出しに対する数値の表示
      result += "\t" + each.getMovie().getTitle() + "\t" +
                String.valueOf(thisAmount)+ "\n";
      totalAmount += thisAmount;
    }
    // フッタ部分の追加
    result += "Amount owed is " + String.valueOf(totalAmount) + "\n";
    result += "You earned " + String.valueOf(frequentRenterPoints) +
              " frequent renter points";
    return result;
  }
}

C++/C#/VB.NETによる リファクタリング – リファクタリング後(Java)

Thursday, April 6th, 2006

Movie.java

// Movie.java

public class Movie {

  public static final int CHILDRENS = 2;
  public static final int REGULAR = 0;
  public static final int NEW_RELEASE = 1;

  private String _title;
  private Price  _price;

  public Movie(String title, int priceCode) {
    _title = title;
    setPriceCode(priceCode);
  }

  public int getPriceCode() {
    return _price.getPriceCode();
  }

  public void setPriceCode(int arg) {
    switch ( arg ) {
    case REGULAR :
      _price = new RegularPrice();
      break;
    case CHILDRENS :
      _price = new ChildrensPrice();
      break;
    case NEW_RELEASE :
      _price = new NewReleasePrice();
      break;
    default:
      throw new IllegalArgumentException("不正な料金コード");
    }
  }

  public String getTitle() {
    return _title;
  }

  public double getCharge(int daysRented) {
    return _price.getCharge(daysRented);
  }

  public int getFrequendRenterPoints(int daysRented) {
    return _price.getFrequentRenterPoints(daysRented);
  }

}

Rental.java

// Rental.java

public class Rental {

  private Movie _movie;
  private int _daysRented;

  public Rental(Movie movie, int daysRented) {
    _movie = movie;
    _daysRented = daysRented;
  }

  public int getDaysRented() {
    return _daysRented;
  }

  public Movie getMovie() {
    return _movie;
  }

  public double getCharge() {
    return _movie.getCharge(_daysRented);
  }

  public int getFrequentRenterPoints() {
    return _movie.getFrequendRenterPoints(_daysRented);
  }

}

Customer.java

// Customer.java

import java.util.*;

public class Customer {

  private String _name;
  private Vector _rentals = new Vector();

  public Customer(String name) {
    _name = name;
  }

  public void addRental(Rental arg) {
    _rentals.addElement(arg);
  }

  public String getName() {
    return _name;
  }

  public String statement() {
    Enumeration rentals = _rentals.elements();
    String result = "Rental Record for " + getName() + "\n";
    while ( rentals.hasMoreElements() ) {
      Rental each = (Rental)rentals.nextElement();
      // この貸し出しに対する数値の表示
      result += "\t" + each.getMovie().getTitle() + "\t" +
                String.valueOf(each.getCharge())+ "\n";
    }
    // フッタ部分の追加
    result += "Amount owed is " + String.valueOf(getTotalCharge()) + "\n";
    result += "You earned " + String.valueOf(getTotalFrequentRenterPoints()) +
              " frequent renter points";
    return result;
  }

  double getTotalCharge() {
    double result = 0;
    Enumeration rentals = _rentals.elements();
    while ( rentals.hasMoreElements() ) {
      Rental each = (Rental)rentals.nextElement();
      result += each.getCharge();
    }
    return result;
  }

  double getTotalFrequentRenterPoints() {
    int result = 0;
    Enumeration rentals = _rentals.elements();
    while ( rentals.hasMoreElements() ) {
      Rental each = (Rental)rentals.nextElement();
      result += each.getFrequentRenterPoints();
    }
    return result;
  }

}

Price.java

// Price.java

public abstract class Price {

  abstract public int getPriceCode();
  abstract public double getCharge(int daysRented);
  public int getFrequentRenterPoints(int daysRented) {
    return 1;
  }
}

class ChildrensPrice extends Price {

  public int getPriceCode() {
    return Movie.CHILDRENS;
  }

  public double getCharge(int daysRented) {
    double result = 1.5;
    if ( daysRented > 3 )
      result += (daysRented -3) * 1.5;
    return result;
  }

}

class NewReleasePrice extends Price {

  public int getPriceCode() {
    return Movie.NEW_RELEASE;
  }

  public double getCharge(int daysRented) {
    return daysRented * 3;
  }

  public int getFrequentRenterPoints(int daysRented) {
    return ( daysRented > 1 ) ? 2 : 1;
  }

}

class RegularPrice extends Price {

  public int getPriceCode() {
    return Movie.REGULAR;
  }

  public double getCharge(int daysRented) {
    double result = 2;
    if ( daysRented > 2 )
      result += (daysRented -2) * 1.5;
    return result;
  }

}

C++/C#/VB.NETによる リファクタリング – リファクタリング前(C++)

Thursday, April 6th, 2006

Movie.h

#ifndef __MOVIE_H__
#define __MOVIE_H__

#include <string>

class Movie {
public:
  enum price_code {
    CHILDRENS = 2,
    REGULAR = 0,
    NEW_RELEASE = 1
  };

  Movie(std::string title, price_code priceCode);
  price_code getPriceCode() const;
  void setPriceCode(price_code arg);
  std::string getTitle() const;

private:
  std::string _title;
  price_code  _priceCode;

};

#endif

Movie.cpp

#include "Movie.h"

Movie::Movie(std::string title, price_code priceCode)
  : _title(title), _priceCode(priceCode) {
}

Movie::price_code Movie::getPriceCode() const {
  return _priceCode;
}

void Movie::setPriceCode(price_code arg) {
  _priceCode = arg;
}

std::string Movie::getTitle() const {
  return _title;
}

Rental.h

#ifndef __RENTAL_H__
#define __RENTAL_H__

class Movie;

class Rental {
public:
  Rental(Movie* movie, int daysRented);
  int getDaysRented() const;
  const Movie* getMovie() const;

private:
  Movie* _movie;
  int _daysRented;

};

#endif

Rental.cpp

#include "Rental.h"

Rental::Rental(Movie* movie, int daysRented) : _movie(movie), _daysRented(daysRented) {
}

int Rental::getDaysRented() const {
  return _daysRented;
}

const Movie* Rental::getMovie() const {
  return _movie;
}

Customer.h

#ifndef __CUSTOMER_H__
#define __CUSTOMER_H__

#include <vector>
#include <string>

class Rental;

class Customer {
public:
  explicit Customer(std::string name);
  void addRental(Rental* arg);
  std::string getName() const;
  std::string statement(double& amount, int& point) const;
  std::string statement() const;

private:
  std::string _name;
  std::vector<Rental*> _rentals;

};

inline std::string Customer::statement() const {
  double amount;
  int point;
  return statement(amount,point);
}

#endif

Customer.cpp

#include <sstream>

#include "Customer.h"
#include "Rental.h"
#include "Movie.h"

Customer::Customer(std::string name) : _name(name) {
}

void Customer::addRental(Rental* arg) {
  _rentals.push_back(arg);
}

std::string Customer::getName() const {
  return _name;
}

std::string Customer::statement(double& totalAmount, int& frequentRenterPoints) const {
  totalAmount = 0;
  frequentRenterPoints = 0;
  std::ostringstream result;
  result << "Rental Record for " << getName() << "\n";
  for ( std::vector<Rental*>::const_iterator rentals = _rentals.begin();
        rentals != _rentals.end(); ++rentals ) {
    double thisAmount = 0;
    const Rental* each = *rentals;
    // 一行ごとに金額を計算
    switch ( each->getMovie()->getPriceCode() ) {
    case Movie::REGULAR :
      thisAmount += 2;
      if ( each->getDaysRented() > 2 )
        thisAmount += (each->getDaysRented() - 2) * 1.5;
      break;
    case Movie::NEW_RELEASE :
      thisAmount += each->getDaysRented() * 3;
      break;
    case Movie::CHILDRENS :
      thisAmount += 1.5;
      if ( each->getDaysRented() > 3 )
        thisAmount += (each->getDaysRented() - 3) * 1.5;
      break;
    }
    // レンタルポイントを加算
    frequentRenterPoints++;
    // 新作を二日以上借りた場合はボーナスポイント
    if ( (each->getMovie()->getPriceCode() == Movie::NEW_RELEASE) &&
         each->getDaysRented() > 1 )
      frequentRenterPoints++;
    // この貸し出しに対する数値の表示
    result << "\t" << each->getMovie()->getTitle() << "\t"
           <<  thisAmount << "\n";
    totalAmount += thisAmount;
  }
  // フッタ部分の追加
  result << "Amount owed is " << totalAmount << "\n"
         << "You earned " << frequentRenterPoints << " frequent renter points";
  return result.str();
}

C++/C#/VB.NETによる リファクタリング – リファクタリング後(C++)

Thursday, April 6th, 2006

Movie.h

#ifndef __MOVIE_H__
#define __MOVIE_H__

#include <string>

class Price;

class Movie {
public:
  enum price_code {
    CHILDRENS = 2,
    REGULAR = 0,
    NEW_RELEASE = 1
  };

  Movie(std::string title, price_code priceCode);
 ~Movie();
  price_code getPriceCode() const;
  void setPriceCode(price_code arg);
  std::string getTitle() const;
  double getCharge(int daysRented) const;
  int getFrequentRenterPoints(int daysRented) const;

private:
  std::string _title;
  Price* _price;

};

#endif

Movie.cpp

#include <stdexcept>

#include "Movie.h"
#include "Price.h"

Movie::Movie(std::string title, price_code priceCode)
  : _title(title), _price(0) {
  setPriceCode(priceCode);
}

Movie::~Movie() {
  delete _price;
}

Movie::price_code Movie::getPriceCode() const {
  return _price->getPriceCode();
}

void Movie::setPriceCode(price_code arg) {
  delete _price;
  switch ( arg ) {
  case REGULAR :
    _price = new RegularPrice();
    break;
  case CHILDRENS :
    _price = new ChildrensPrice();
    break;
  case NEW_RELEASE :
    _price = new NewReleasePrice();
    break;
  default:
    throw std::runtime_error("不正な料金コード");
  }
}

std::string Movie::getTitle() const {
  return _title;
}

double Movie::getCharge(int daysRented) const {
  return _price->getCharge(daysRented);
}

int Movie::getFrequentRenterPoints(int daysRented) const {
  return _price->getFrequentRenterPoints(daysRented);
}

Rental.h

#ifndef __RENTAL_H__
#define __RENTAL_H__

#include "Movie.h"

class Rental {
public:
  Rental(Movie* movie, int daysRented);
  int getDaysRented() const;
  const Movie* getMovie() const;
  double getCharge() const;
  int getFrequentRenterPoints() const;

private:
  Movie* _movie;
  int _daysRented;

};

#endif

Rental.cpp

#include "Rental.h"

Rental::Rental(Movie* movie, int daysRented) : _movie(movie), _daysRented(daysRented) {
}

int Rental::getDaysRented() const {
  return _daysRented;
}

const Movie* Rental::getMovie() const {
  return _movie;
}

double Rental::getCharge() const {
  return _movie->getCharge(_daysRented);
}

int Rental::getFrequentRenterPoints() const {
  return _movie->getFrequentRenterPoints(_daysRented);
}

Customer.h

#ifndef __CUSTOMER_H__
#define __CUSTOMER_H__

#include <vector>
#include <string>

class Rental;

class Customer {
  friend class CustomerTest;

public:
  explicit Customer(std::string name);
  void addRental(Rental* arg);
  std::string getName() const;
  std::string statement() const;

private:
  double getTotalCharge() const;
  int getTotalFrequentRenterPoints() const;

  std::string _name;
  std::vector<Rental*> _rentals;

};

#endif

Customer.cpp

#include <sstream>

#include "Customer.h"
#include "Rental.h"
#include "Movie.h"

Customer::Customer(std::string name) : _name(name) {
}

void Customer::addRental(Rental* arg) {
  _rentals.push_back(arg);
}

std::string Customer::getName() const {
  return _name;
}

std::string Customer::statement() const {
  std::ostringstream result;
  result << "Rental Record for " << getName() << "\n";
  for ( std::vector<Rental*>::const_iterator rentals = _rentals.begin();
        rentals != _rentals.end(); ++rentals ) {
    const Rental* each = *rentals;
    // この貸し出しに対する数値の表示
    result << "\t" << each->getMovie()->getTitle() << "\t"
           <<  each->getCharge() << "\n";
  }
  // フッタ部分の追加
  result << "Amount owed is " << getTotalCharge() << "\n"
         << "You earned " << getTotalFrequentRenterPoints() << " frequent renter points";
  return result.str();
}

double Customer::getTotalCharge() const {
  double result = 0;
  for ( std::vector<Rental*>::const_iterator rentals = _rentals.begin();
        rentals != _rentals.end(); ++rentals ) {
    const Rental* each = *rentals;
    result += each->getCharge();
  }
  return result;
}

int Customer::getTotalFrequentRenterPoints() const {
  int result = 0;
  for ( std::vector<Rental*>::const_iterator rentals = _rentals.begin();
        rentals != _rentals.end(); ++rentals ) {
    const Rental* each = *rentals;
    result += each->getFrequentRenterPoints();
  }
  return result;
}

Price.h

#ifndef __PRICE_H__
#define __PRICE_H__

#include "Movie.h"

class Price {
public:
  virtual ~Price() {}
  virtual Movie::price_code getPriceCode() const =0;
  virtual double getCharge(int daysRented) const =0;
  virtual int getFrequentRenterPoints(int daysRented) const;
};

class ChildrensPrice : public Price {
public:
  virtual Movie::price_code getPriceCode() const;
  virtual double getCharge(int daysRented) const;
};

class NewReleasePrice : public Price {
public:
  virtual Movie::price_code getPriceCode() const;
  virtual double getCharge(int daysRented) const;
  virtual int getFrequentRenterPoints(int daysRented) const;
};

class RegularPrice : public Price {
public:
  virtual Movie::price_code getPriceCode() const;
  virtual double getCharge(int daysRented) const;
};

#endif

Price.cpp

#include "Price.h"

// Price

int Price::getFrequentRenterPoints(int daysRented) const {
  return 1;
}

// ChildrensPrice

Movie::price_code ChildrensPrice::getPriceCode() const {
  return Movie::CHILDRENS;
}

double ChildrensPrice::getCharge(int daysRented) const {
  double result = 1.5;
  if ( daysRented > 3 )
    result += (daysRented - 3) * 1.5;
  return result;
}

// NewReleasePrice

Movie::price_code NewReleasePrice::getPriceCode() const {
  return Movie::NEW_RELEASE;
}

double NewReleasePrice::getCharge(int daysRented) const {
  return daysRented * 3;
}

int NewReleasePrice::getFrequentRenterPoints(int daysRented) const {
  return  ( daysRented > 1 ) ? 2 : 1;
}

// RegularPrice

Movie::price_code RegularPrice::getPriceCode() const {
  return Movie::REGULAR;
}

double RegularPrice::getCharge(int daysRented) const {
  double result = 2;
  if ( daysRented > 2 )
    result += (daysRented - 2) * 1.5;
  return result;
}

C++/C#/VB.NETによる リファクタリング – リファクタリング前(C#)

Thursday, April 6th, 2006

Movie.cs

// Movie.cs

namespace csharp_before {

  public class Movie {

    public enum enumPriceCode {
      CHILDRENS = 2, REGULAR = 0, NEW_RELEASE = 1
    };

    private string _title;
    private enumPriceCode _priceCode;

    public Movie(string title, enumPriceCode priceCode) {
      _title = title;
      _priceCode = priceCode;
    }

    public enumPriceCode PriceCode {
      get { return _priceCode; }
      set { _priceCode = value; }
    }

    public string Title {
      get { return _title; }
    }

  }

}

Rental.cs

// Rental.cs

namespace csharp_before {

  public class Rental {

    private Movie _movie;
    private int _daysRented;

    public Rental(Movie movie, int daysRented) {
      _movie = movie;
      _daysRented = daysRented;
    }

    public int DaysRented {
      get { return _daysRented; }
    }

    public Movie Movie {
      get { return _movie; }
    }

  }

}

Customer.cs

// Customer.cs

namespace csharp_before {

  public class Customer {

    private string _name;
    private System.Collections.ArrayList _rentals;

    public Customer(string name) {
      _rentals = new System.Collections.ArrayList();
      _name = name;
    }

    public void addRental(Rental arg) {
      _rentals.Add(arg);
    }

    public string Name {
      get { return _name; }
    }

    public string statement() {
      double totalAmount = 0;
      int frequentRenterPoints = 0;
      string result = "Rental Record for " + Name + "\n";
      foreach ( Rental each in _rentals ) {
        double thisAmount = 0;
        // 一行ごとに金額を計算
        switch ( each.Movie.PriceCode ) {
        case Movie.enumPriceCode.REGULAR :
          thisAmount += 2;
          if ( each.DaysRented > 2 )
            thisAmount += (each.DaysRented - 2) * 1.5;
          break;
        case Movie.enumPriceCode.NEW_RELEASE :
          thisAmount += each.DaysRented * 3;
          break;
        case Movie.enumPriceCode.CHILDRENS :
          thisAmount += 1.5;
          if ( each.DaysRented > 3 )
            thisAmount += (each.DaysRented - 3) * 1.5;
          break;
        }
        // レンタルポイントを加算
        frequentRenterPoints++;
        // 新作を二日以上借りた場合はボーナスポイント
        if ( (each.Movie.PriceCode == Movie.enumPriceCode.NEW_RELEASE) &&
              each.DaysRented > 1 )
          frequentRenterPoints++;
        // この貸し出しに対する数値の表示
        result += "\t" + each.Movie.Title + "\t" +
                  thisAmount.ToString() + "\n";
        totalAmount += thisAmount;
      }
      // フッタ部分の追加
      result += "Amount owed is " + totalAmount.ToString() + "\n";
      result += "You earned " + frequentRenterPoints.ToString() +
                " frequent renter points";
      return result;
    }
  }
}

C++/C#/VB.NETによる リファクタリング – リファクタリング後(C#)

Thursday, April 6th, 2006

Movie.cs

// Movie.cs

namespace csharp_after {

  public class Movie {

    public enum enumPriceCode {
      CHILDRENS = 2, REGULAR = 0, NEW_RELEASE = 1
    }

    private string _title;
    private Price  _price;

    public Movie(string title, enumPriceCode priceCode) {
      _title = title;
      PriceCode = priceCode;
    }

    public enumPriceCode PriceCode {
      get { return _price.PriceCode; }
      set {
        switch ( value ) {
        case Movie.enumPriceCode.REGULAR :
          _price = new RegularPrice();
          break;
        case Movie.enumPriceCode.CHILDRENS :
          _price = new ChildrensPrice();
          break;
        case Movie.enumPriceCode.NEW_RELEASE :
          _price = new NewReleasePrice();
          break;
        default:
          throw new System.Exception("不正な料金コード");
        }
      }
    }

     public string Title {
       get { return _title; }
     }

     public double getCharge(int daysRented) {
       return _price.getCharge(daysRented);
     }

     public int getFrequentRenterPoints(int daysRented) {
       return _price.getFrequentRenterPoints(daysRented);
     }

  }
}

Rental.cs

// Rental.cs

namespace csharp_after {

  public class Rental {

    private Movie _movie;
    private int _daysRented;

    public Rental(Movie movie, int daysRented) {
      _movie = movie;
      _daysRented = daysRented;
    }

    public int DaysRented {
      get { return _daysRented; }
    }

    public Movie Movie {
      get { return _movie; }
    }

    public double Charge {
      get { return _movie.getCharge(_daysRented); }
    }

    public int FrequentRenterPoints {
      get { return _movie.getFrequentRenterPoints(_daysRented); }
    }

  }

}

Customer.cs

// Customer.cs

namespace csharp_after {

  public class Customer {

    private string _name;
    private System.Collections.ArrayList _rentals;

    public Customer(string name) {
      _rentals = new System.Collections.ArrayList();
      _name = name;
    }

    public void addRental(Rental arg) {
      _rentals.Add(arg);
    }

    public string Name {
      get { return _name; }
    }

    public string statement() {
      string result = "Rental Record for " + Name + "\n";
      foreach ( Rental each in _rentals ) {
        // この貸し出しに対する数値の表示
        result += "\t" + each.Movie.Title + "\t" +
                  each.Charge.ToString() + "\n";
      }
      // フッタ部分の追加
      result += "Amount owed is " + TotalCharge.ToString() + "\n";
      result += "You earned " + TotalFrequentRenterPoints.ToString() +
                " frequent renter points";
      return result;
    }

    double TotalCharge {
      get {
        double result = 0;
        foreach ( Rental each in _rentals ) {
          result += each.Charge;
        }
        return result;
      }
    }

    double TotalFrequentRenterPoints {
      get {
        int result = 0;
        foreach ( Rental each in _rentals ) {
          result += each.FrequentRenterPoints;
        }
        return result;
      }
    }
  }

}

Price.cs

// Price.cs

namespace csharp_after {

  public abstract class Price {

    abstract public Movie.enumPriceCode PriceCode { get; }
    abstract public double getCharge(int daysRented);
    virtual public int getFrequentRenterPoints(int daysRented) { return 1; }
  }

  class ChildrensPrice : Price {

    public override Movie.enumPriceCode PriceCode {
      get { return Movie.enumPriceCode.CHILDRENS; }
    }

    public override double getCharge(int daysRented) {
      double result = 1.5;
      if ( daysRented > 3 )
        result += (daysRented -3) * 1.5;
      return result;
    }

  }

  class NewReleasePrice : Price {

    public override Movie.enumPriceCode PriceCode {
      get { return Movie.enumPriceCode.NEW_RELEASE; }
    }

    public override double getCharge(int daysRented) {
      return daysRented * 3;
    }

    public override int getFrequentRenterPoints(int daysRented) {
      return ( daysRented > 1 ) ? 2 : 1;
    }

  }

  class RegularPrice : Price {

    public override Movie.enumPriceCode PriceCode {
      get { return Movie.enumPriceCode.REGULAR; }
    }

    public override double getCharge(int daysRented) {
      double result = 2;
      if ( daysRented > 2 )
        result += (daysRented -2) * 1.5;
      return result;
    }
  }

}

C++/C#/VB.NETによる リファクタリング – リファクタリング前(VB.NET)

Thursday, April 6th, 2006

Movie.vb

Namespace visualbasic_before

  Public Class Movie

    Public Enum enumPriceCode
      CHILDRENS = 2
      REGULAR = 0
      NEW_RELEASE = 1
    End Enum

    Private _title As String
    Private _priceCode As enumPriceCode

    Public Sub New(ByVal title As String, ByVal priceCode As enumPriceCode)
      _title = title
      _priceCode = priceCode
    End Sub

    Public ReadOnly Property PriceCode() As enumPriceCode
      Get
        PriceCode = _priceCode
      End Get
    End Property

    Public ReadOnly Property Title() As String
      Get
        Title = _title
      End Get
    End Property

  End Class

End Namespace

Rental.vb

Namespace visualbasic_before

  Public Class Rental

    Private _movie As Movie
    Private _daysRented As Integer

    Public Sub New(ByVal movie As Movie, ByVal daysRented As Integer)
      _movie = movie
      _daysRented = daysRented
    End Sub

    Public ReadOnly Property DaysRented() As Integer
      Get
        DaysRented = _daysRented
      End Get
    End Property

    Public ReadOnly Property Movie() As Movie
      Get
        Movie = _movie
      End Get
    End Property

  End Class

End Namespace

Customer.vb

Namespace visualbasic_before

  Public Class Customer

    Private _name As String
    Private _rentals As System.Collections.ArrayList

    Public Sub New(ByVal name As String)
      _rentals = New System.Collections.ArrayList()
      _name = name
    End Sub

    Public Sub addRental(ByVal arg As Rental)
      _rentals.Add(arg)
    End Sub

    Public ReadOnly Property Name() As String
      Get
        Name = _name
      End Get
    End Property

    Public Function statement() As String
      Dim totalAmount As Double
      Dim frequentRenterPoints As Integer
      Dim result As String
      Dim item As Rental
      totalAmount = 0
      frequentRenterPoints = 0
      result = "Rental Record for " + Name + Chr(13)
      For Each item In _rentals
        Dim thisAmount As Double
        thisAmount = 0
        ' 一行ごとに金額を計算
        Select Case item.Movie.PriceCode
        Case Movie.enumPriceCode.REGULAR
          thisAmount += 2
          If item.DaysRented > 2 Then
            thisAmount += (item.DaysRented - 2) * 1.5
          End If
        Case Movie.enumPriceCode.NEW_RELEASE
          thisAmount += item.DaysRented * 3
        Case Movie.enumPriceCode.CHILDRENS
          thisAmount += 1.5
          If item.DaysRented > 3 Then
            thisAmount += (item.DaysRented - 3) * 1.5
          End If
        End Select
        ' レンタルポイントを加算
        frequentRenterPoints = frequentRenterPoints + 1
        ' 新作を二日以上借りた場合はボーナスポイント
        If item.Movie.PriceCode = Movie.enumPriceCode.NEW_RELEASE And item.DaysRented > 1 Then
          frequentRenterPoints = frequentRenterPoints + 1
        End If
        ' この貸し出しに対する数値の表示
        result += Chr(9) + item.Movie.Title + Chr(9) + thisAmount.ToString() + Chr(13) + Chr(10)
        totalAmount += thisAmount
      Next
      ' フッタ部分の追加
      result += "Amount owed is " + totalAmount.ToString() + Chr(13) + Chr(10)
      result += "You earned " + frequentRenterPoints.ToString() + " frequent renter points"

      statement = result
    End Function
End Class

End Namespace

C++/C#/VB.NETによる リファクタリング – リファクタリング後(VB.NET)

Thursday, April 6th, 2006

Movie.vb

Namespace visualbasic_after

  Public Class Movie

    Public Enum enumPriceCode
      CHILDRENS = 2
      REGULAR = 0
      NEW_RELEASE = 1
    End Enum

    Private _title As String
    Private _price As Price

    Public Sub New(ByVal title As String, ByVal code As enumPriceCode)
      _title = title
      PriceCode = code
    End Sub

    Public Property PriceCode() As enumPriceCode
      Get
        PriceCode = _price.PriceCode
      End Get
      Set(ByVal Value As enumPriceCode)
        Select Case Value
        Case Movie.enumPriceCode.REGULAR
          _price = New RegularPrice()
        Case Movie.enumPriceCode.CHILDRENS
          _price = New ChildrensPrice()
        Case Movie.enumPriceCode.NEW_RELEASE
          _price = New NewReleasePrice()
        Case Else
          Throw New System.Exception("不正な料金コード")
        End Select
      End Set
    End Property

    Public ReadOnly Property Title() As String
      Get
        Title = _title
      End Get
    End Property

    Public Function getCharge(ByVal daysRented As Integer) As Double
       getCharge = _price.getCharge(daysRented)
    End Function

    Public Function getFrequentRenterPoints(ByVal daysRented As Integer) As Integer
       getFrequentRenterPoints = _price.getFrequentRenterPoints(daysRented)
    End Function

  End Class

End Namespace

Rental.vb

Namespace visualbasic_after

  Public Class Rental

    Private _movie As Movie
    Private _daysRented As Integer

    Public Sub New(ByVal movie As Movie, ByVal daysRented As Integer)
      _movie = movie
      _daysRented = daysRented
    End Sub

    Public ReadOnly Property DaysRented() As Integer
      Get
        DaysRented = _daysRented
      End Get
    End Property

    Public ReadOnly Property TheMovie() As Movie
      Get
        TheMovie = _movie
      End Get
    End Property

    Public ReadOnly Property Charge() As Double
      Get
        Charge = _movie.getCharge(_daysRented)
      End Get
    End Property

    Public ReadOnly Property FrequentRenterPoints() As Integer
      Get
        FrequentRenterPoints = _movie.getFrequentRenterPoints(_daysRented)
      End Get
    End Property

  End Class

End Namespace

Customer.vb

Namespace visualbasic_after

  Public Class Customer

    Private _name As String
    Private _rentals As System.Collections.ArrayList

    Public Sub New(ByVal name As String)
      _rentals = New System.Collections.ArrayList()
      _name = name
    End Sub

    Public Sub addRental(ByVal arg As Rental)
      _rentals.Add(arg)
    End Sub

    Public ReadOnly Property Name() As String
      Get
        Name = _name
      End Get
    End Property

    Public Function statement() As String
      Dim result As String = "Rental Record for " + Name + Chr(13) + Chr(10)
      Dim item As Rental
      For Each item In _rentals
        ' この貸し出しに対する数値の表示
        result += Chr(9) + item.TheMovie.Title + Chr(9) + item.Charge.ToString() + Chr(13) + Chr(10)
      Next
      ' フッタ部分の追加
      result += "Amount owed is " + TotalCharge.ToString() + Chr(13) + Chr(10)
      result += "You earned " + TotalFrequentRenterPoints.ToString() + " frequent renter points"
      statement = result
    End Function

    Public ReadOnly Property TotalCharge() As Double
      Get
        Dim result As Double = 0
        Dim item As Rental
        For Each item In _rentals
          result += item.Charge
        Next
        TotalCharge = result
      End Get
    End Property

    Public ReadOnly Property TotalFrequentRenterPoints() As Double
      Get
        Dim result As Integer = 0
        Dim item As Rental
        For Each item In _rentals
          result += item.FrequentRenterPoints
        Next
        TotalFrequentRenterPoints = result
      End Get
    End Property
  End Class

End Namespace

Price.vb

Namespace visualbasic_after

  Public MustInherit Class Price

    Public ReadOnly Property PriceCode() As Movie.enumPriceCode
      Get
        PriceCode = getPriceCode()
      End Get
    End Property

    Public MustOverride Function getCharge(ByVal daysRented As Integer) As Double

    Public Overridable Function getFrequentRenterPoints(ByVal daysRented As Integer) As Integer
      getFrequentRenterPoints = 1
    End Function

    Public Overridable Function getPriceCode() As Movie.enumPriceCode
      getPriceCode = Movie.enumPriceCode.REGULAR
    End Function

  End Class

  Class ChildrensPrice
    Inherits Price

    Public Overrides Function getPriceCode() As Movie.enumPriceCode
      getPriceCode = Movie.enumPriceCode.CHILDRENS
    End Function

    Public Overrides Function getCharge(ByVal daysRented As Integer) As Double
      Dim result As Double = 1.5
      If daysRented > 3 Then
        result += (daysRented - 3) * 1.5
      End If
      getCharge = result
    End Function

  End Class

  Public Class NewReleasePrice
    Inherits Price

    Public Overrides Function getPriceCode() As Movie.enumPriceCode
      getPriceCode = Movie.enumPriceCode.NEW_RELEASE
    End Function

    Public Overrides Function getCharge(ByVal daysRented As Integer) As Double
      getCharge = daysRented * 3
    End Function

    Public Overrides Function getFrequentRenterPoints(ByVal daysRented As Integer) As Integer
      If daysRented > 1 Then
        getFrequentRenterPoints = 2
      Else
        getFrequentRenterPoints = 1
      End If
    End Function

  End Class

  Class RegularPrice
    Inherits Price

    Public Overrides Function getPriceCode() As Movie.enumPriceCode
      getPriceCode = Movie.enumPriceCode.REGULAR
    End Function

    Public Overrides Function getCharge(ByVal daysRented As Integer) As Double
      Dim result As Double = 2
      If daysRented > 2 Then
        result += (daysRented - 2) * 1.5
      End If
      getCharge = result
    End Function

  End Class

End Namespace