2017-06-07 9 views
0

私のプログラムはかなり大きくなってしまい、ちょうど短縮できるコードがたくさんありません。このコードを短縮する効率的な方法があるかどうか疑問に思う

private void bookComboBox_SelectedIndexChanged(object sender, EventArgs e) 
{ 
    string books = null; 
    // sets books to the clicked item 
    books = bookComboBox.SelectedItem.ToString(); 
    selectedPictureBox.Visible = true; 
    // Loads string to list box and image to selectedPictureBox when programming is selected 
    if (books == "Programming") 
    { 
     bookListBox.Items.Clear(); 
     selectedPictureBox.Image = Image.FromFile("programming.png"); 
     bookListBox.Items.Add("Visual Basic"); 
     bookListBox.Items.Add("Java"); 
     bookListBox.Items.Add("C#"); 
    } 
    // Loads string to list box and image to selectedPictureBox when Networking is selected 
    else if (books == "Networking") 
    { 
     bookListBox.Items.Clear(); 
     selectedPictureBox.Image = Image.FromFile("networking.png"); 
     bookListBox.Items.Add("LAN Networks"); 
     bookListBox.Items.Add("Windows Networking"); 
     bookListBox.Items.Add("More About Networking"); 
    } 
    // Loads string to list box and image to selectedPictureBox when Web is selected 
    else if (books == "Web") 
    { 
     bookListBox.Items.Clear(); 
     selectedPictureBox.Image = Image.FromFile("html.png"); 
     bookListBox.Items.Add("Web Programming"); 
     bookListBox.Items.Add("JavaScript"); 
     bookListBox.Items.Add("ASP"); 
    } 
} 

コードが正常に動作しますが、私はただ、必要であれば、このコードを短縮する上でいくつかのヒントを得るために期待していた、いずれかの入力が高く評価されています。ここで私は上のいくつかのヒントを探していますつのインスタンスです。

+1

データストアを使用してアイテムを格納することをお勧めします。キーが本の種類で、リストボックスを構築するためにそのデータをプルする辞書のようなもの – Colwin

+0

はい。まずif-elseの外でbookListBox.Items.Clear()を取り出します。状態が始まる前に置く。 2番目の画像ファイル名のみを格納する変数を宣言し、この変数名を各条件のファイル名に設定し、このファイル名を条件外のpictureboxに割り当てます。 3番目に、ブックアイテムを格納するリストを作成し、bookListBoxデータソースをそれらのリストに割り当てます。 –

+0

@BhubanShresthaサンプルコードでは、これらのオプションと一致しないコンボボックス項目が存在する可能性があるため、リスト外では移動しません。 –

答えて

2

あなたがC#7の新しいタプルを使用することができますと仮定:

private Dictionary<string, (string image, List<string> books)> books = new Dictionary<string, (string image, List<string> books)> 
{ 
    { "Programming", ("programming.png", new List<string> { "Visual Basic", "Java", "C#"}) }, 
    { "Networking", ("networking.png", new List<string> {"LAN Networks", "Windows Networking", "More About Networking"}) }, 
    { "Web", ("html.png", new List<string> {"Web Programming", "Javascript", "ASP"}) } 
}; 

private void bookComboBox_SelectedIndexChanged(object sender, EventArgs e) 
{ 
    // sets books to the clicked item 
    string book = bookComboBox.SelectedItem.ToString(); 
    selectedPictureBox.Visible = true; 

    if (books.Keys.Contains(book)) 
    { 
     bookListBox.Items.Clear(); 
     selectedPictureBox.Image = Image.FromFile(books[book].image); 
     foreach(var b in books[book].books) 
     { 
      bookListBox.Items.Add(b); 
     } 
    } 
} 

しかし、クラスはさらに良いそうです:

すべて今使用して、異なるではありませんが、それは形式化し
public class BookGroup 
{ 
    public string ImagePath {get;set;} 
    public List<string> Books {get;} 

    public BookGroup(string imagePath, params string[] books) 
    { 
     ImagePath = imagePath; 
     Books = new List<string>(books.Length); 
     Books.AddRange(books); 
    } 
} 

このコードを道路で簡単に使用できるようにするものがいくつかあります。タプルをまだ使用できない場合は可能です。また、私はちょうど楽しみのために、今のところ、それ自体でBookクラスを持っているかもしれません:

private Dictionary<string, BookGroup> books = new Dictionary<string, BookGroup> 
{ 
    { "Programming", new BookGroup("programming.png", "Visual Basic", "Java", "C#")}, 
    { "Networking", new BookGroup("networking.png","LAN Networks", "Windows Networking", "More About Networking") }, 
    { "Web", new BookGroup("html.png", "Web Programming", "Javascript", "ASP") } 
}; 

private void bookComboBox_SelectedIndexChanged(object sender, EventArgs e) 
{ 
    // sets books to the clicked item 
    string book = bookComboBox.SelectedItem.ToString(); 
    selectedPictureBox.Visible = true; 

    if (books.Keys.Contains(book)) 
    { 
     bookListBox.Items.Clear(); 
     BookGroup bg = books[book]; 
     selectedPictureBox.Image = Image.FromFile(bg.ImagePath); 
     foreach(var b in bg.Books) 
     { 
      bookListBox.Items.Add(b); 
     } 
    } 
} 

かかわらず、私は間違いなくは、テキストファイルからこれらをロードする方法を持ってをいただきたい...そうCSV、新しいプロセスコードを再コンパイルまたは配布しなくてもこのリストを更新することができます。そのことを念頭に置いて、このデータを1つのファイルにまとめるために、画像を繰り返して各書籍を入力すると、CSVデータは次のようになります。

 
Topic,Image,Title 
Programming,programming.png,"Visual Basic" 
Programming,programming.png,"Java" 
Programming,programming.png,"C#" 
Networking,networking.png,"LAN Networks" 
Networking,networking.png,"Windows Networking" 
Networking,networking.png,"More About Networking" 
Web,html.png,"Web Programming" 
Web,html.png,"Javascript" 
Web,html.png,"ASP"

これは、コードの文字全体を変更します。私は少しバイアスされんだけど、私はそうthis CSV Parserを使用したい、そして再び私はこのような何かを生産したいタプルを仮定:

private List<(string Topic, string ImagePath, string Title)> books; 

//In the form load code: 
books = EasyCSV.FromFile("bookData.csv").Select(b => (b[0], b[1], b[2])).ToList(); 

//and finally, in the original selectindexchanged method: 
private void bookComboBox_SelectedIndexChanged(object sender, EventArgs e) 
{ 
    string topic = bookComboBox.SelectedItem.ToString(); 
    selectedPictureBox.Visible = true; 

    var items = books.Where(b => b.Topic == topic).ToArray(); 
    if(items.Length > 0) 
    { 
     bookListBox.Items.Clear(); 
     selectedPictureBox.Image = Image.FromFile(items[0].ImagePath); 
     bookListBox.Items.AddRange(items); 
    } 
} 
+0

詳細をお寄せいただきありがとうございます。これはうまく動作し、間違いなくクラスを実装しようとしていました。 – KobiashiMaru

+0

あなたは私のためにこれを見て、あなたに何か提案があれば教えてください。 https://stackoverflow.com/questions/44479418/i-am-trying-to-add-items-from-combox-to-listview-that-equal-the-contents-of-a-di/44479456#44479456 – KobiashiMaru

1

コードの長さについては、switch-case-break-defaultの構文を使用することをお勧めします。 books変数を切り替えます。 これはパフォーマンスを改善するものではありませんが

1

私はビジュアルスタジオを持っていないので、改善のポイント/提案をしています。

  • switchは、if-elseifよりも好ましいはずです。
  • bookListBox.Items.Clear();およびs electedPictureBox.Image,ifブロック。変数を使用して画像ファイル名を設定します。
1

書籍カテゴリを表すクラスを作成する必要があります。その後、あなたは、単にこのように、必要な情報をすべてのカテゴリリストを反復処理し、抽出できます。そして、これらを保持するために辞書を作成

public class BookList 
{ 
    public string ImageName { get; set; } 
    public List<string> Items { get;set; } 
} 

string books = null; 
books = bookComboBox.SelectedItem.ToString(); 
selectedPictureBox.Visible = true; 

for (int i = 0; i < categories.Count; i++) { 
    if (books == categories[i].Name) { 
     bookListBox.Items.Clear(); 
     selectedPictureBox.Image = Image.FromFile(categories[i].ImagePath); 
     for (int j = 0; j < categories[i].Items.Count; j++) { 
      bookListBox.Items.Add(categories[i].Items[j]); 
     }   
    } 
} 
1

は本のリストを表すクラスを作成します。アイテム:次に

Dictionary<string, BookList> bookLists = new Dictionary<string, BookList> 
    { 
    { 
     "Programming", 
     new BookList { ImageName = "programming.png", Items = new List<string> { ... } } 
    } 
    ... 
    }; 

にあなたのif文を変更します。

if (bookLists.ContainsKey(books)) 
{ 
    bookListBox.Items.Clear(); 
    selectedPictureBox.Image = Image.FromFile(bookLists[books].ImageName); 
    foreach (var b in bookLists[books].Items) 
    { 
    bookListBox.Items.Add(b); 
    } 
} 
+0

ありがとう、これは私が探していたものでした。 – KobiashiMaru

1

すべてのデータを構成オブジェクトに保存し、チェックと割り当てを実行するときにそのデータを繰り返し処理することをお勧めします。

名前、画像ファイル名、チェックボックス項目の文字列配列など、各書籍のデータを保持する別のクラスを作成します。

私はそのオブジェクトのリストを作成し、フォームの初期化時に手動ですべてのデータを割り当てます。

その後、SelectedIndexChangedイベントハンドラで、各アイテムのforループを繰り返し、ブック名が一致するかどうかを確認します。それができたら、そのオブジェクトのデータを使用してから、ループbreak;を使用します。

2

オブジェクトを作成し、データバインディングを使用します。

public class Book 
{ 
    public BookType BookType { get; set; } 
    public string Name { get; set; } 
    public string Image { get; set; } 
} 

public enum BookType 
{ 
    Programming, 
    Networking, 
    Web, 
} 

public partial class Form1 : Form 
{ 
    private readonly List<Book> _books = new List<Book> 
    { 
     new Book { Image = "programming.png", BookType = BookType.Programming, Name = "VB" }, 
     new Book { Image = "programming.png", BookType = BookType.Programming, Name = "Java" }, 
     new Book { Image = "programming.png", BookType = BookType.Programming, Name = "C#" }, 
     new Book { Image = "networking.png", BookType = BookType.Networking, Name = "LAN Networks" }, 
     new Book { Image = "networking.png", BookType = BookType.Networking, Name = "Windows Networking" }, 
     new Book { Image = "networking.png", BookType = BookType.Networking, Name = "More About Networking" }, 
     new Book { Image = "html.png", BookType = BookType.Web, Name = "Web Programming" }, 
     new Book { Image = "html.png", BookType = BookType.Web, Name = "Javascript" }, 
     new Book { Image = "html.png", BookType = BookType.Web, Name = "ASP" }, 
    }; 

    public Form1() 
    { 
     InitializeComponent(); 
    } 

    private void Form1_Load(object sender, EventArgs e) 
    { 
     var bookTypes = _books.GroupBy(b => b.BookType).Select(g => g.Key).ToList(); 

     this.cboBookTypes.DataSource = bookTypes; 
    } 

    private void cboBookTypes_SelectedIndexChanged(object sender, EventArgs e) 
    { 
     var bookType = (BookType)this.cboBookTypes.SelectedItem; 
     var books = _books.Where(b => b.BookType == bookType).ToList(); 
     var img = books.First().Image; 

     this.imgBook.Image = Image.FromFile(img); 
     this.lstBooks.DataSource = books; 
     this.lstBooks.DisplayMember = "Name"; 

    } 
} 
関連する問題