2010-11-14 132 views
1

我有一個構造的方法來創建一個對象

public Track(string path) 
{ 
     if (!File.Exists(path)) 
      throw new FileNotFoundException("File not found", path); 
     if (!IsAudioFile(path)) 
      throw new Exception("Illegal Audio Format"); 

     _path = path; 
     _id = Guid.NewGuid(); 
     _rate = 0; 
     _length = GetTrackLength(path); 

     TagLib.File file = TagLib.File.Create(path); 
     if (!file.Tag.IsEmpty) 
     { 
      try 
      { 
       _artist = file.Tag.Artists[0]; 
      } 
      catch (Exception e) 
      { 
       _artist = ""; 
      } 
      _title = file.Tag.Title; 
      try 
      { 
       _genre = file.Tag.Genres[0].ToGenre(); 
      } 
      catch (Exception e) 
      { 
       _genre = Genre.NoGenre; 
      } 
     } 
     else 
     { 
      _artist = "Unknown"; 
      _title = "Unknown"; 
      _genre = Genre.NoGenre; 
     } 
} 

我應該拋出一個異常,或者我應該選擇創建對象的另一種方式? 例如:

Track track = new Track(path);
track = Track.GetInstance();

回答

0

我會做一些小的修改,如下所示,以避免不必要的異常處理,並且還會在參數檢查中拋出一個更具體的異常。就投入構造函數而言,這很好,因爲它實際上只是特殊類型的方法。

關於創建新的Track的反饋:你可以放置一個GetTrack()方法在某種經理(TagManager爲例),如果你認爲new'ing了Track對象是什麼,你的API應該將處理的給對消費者負責。

public Track(string path) 
{ 
    if (!File.Exists(path)) 
     throw new FileNotFoundException("File not found", path); 
    if (!IsAudioFile(path)) 
     throw new InvalidOperationException("Illegal Audio Format"); 

    _path = path; 
    _id = Guid.NewGuid(); 
    _rate = 0; 
    _length = GetTrackLength(path); 

    TagLib.File file = TagLib.File.Create(path); 
    if (!file.Tag.IsEmpty) 
    { 
     _title = file.Tag.Title; 

     if (file.Tag.Artists != null && file.Tag.Artists.Count > 0) 
      _artist = file.Tag.Artists[0]; 
     else 
      _artist = ""; 


     if (file.Tag.Genres != null && file.Tag.Genres.Count > 0) 
      _genre = file.Tag.Genres[0].ToGenre(); 
     else 
      _genre = Genre.NoGenre; 
    } 
    else 
    { 
     _artist = "Unknown"; 
     _title = "Unknown"; 
     _genre = Genre.NoGenre; 
    } 
} 
1

你的代碼是正確的,圖案很好。

但是,you shouldn't throw the base Exception class
相反,您應該拋出一個ArgumentException,InvalidDataExceptionInvalidOperationException。您也可以shouldn't catch the base Exception class
取而代之,您應該捕獲ToGenre方法可能拋出的任何異常。

+0

你認爲我做的一切正確? – Sergey 2010-11-14 15:14:31

+0

更好使用ArgumentException,我認爲 – 2010-11-14 15:15:02

+0

@Segey:差不多。 – SLaks 2010-11-14 15:15:46

0

有(通常)沒有理由有靜態方法返回實例除非你正在做一個單身模式或需要重新使用現有的對象,而不是創建新的實例。

在構造函數中拋出異常很好,而且通常是正確的做事方式。

+0

我認爲構造函數中的異常不是一個好的決定 – Sergey 2010-11-14 15:17:12

+1

@Sergey:在構造函數中拋出異常沒有任何問題。 http://msdn.microsoft.com/en-us/library/bb386039。aspx普通(非'靜態')構造函數不在列表中。 – SLaks 2010-11-14 15:18:06

+1

過去有些語言在構造中拋出異常(有時)是不好的。 C#不是其中之一。 – Donnie 2010-11-14 15:24:37

0

代碼很酷。

  1. 您在開始工作之前檢查參數。
  2. 您的代碼清晰易讀
  3. 您處理沒有提供信息時設置默認值的情況。

我所能建議的不是使用泛型異常,而是拋出並捕獲更具體的上下文。隨意使用ArgumentException。

而且,只是CheckStyle的:

是,如果你想在若{}塊添加代碼,使用大括號{}始終如果是一個好的做法,你可以忘記把括號。雖然這裏沒有必要。

相關問題