Searching for an object with spectific property, using user input

Question

I have just started my adventure with C#. Below you can find a code written for my school assigment.

The homework task had 3 major elements:

1. building a class book
2. creating a metod for changing the static genre value and
3. creating a way to find specific object by properties.

All those elements are included.

I hope that more experienced members of community will have a short look at my code and share some tips and/or good practices, so I may improve.

class Book
{
 public string title { get; set; }
 public string Author { get; set; }
 public string publisher { get; set; }
 public int price { get; set; }
 public static string genre;
 public override string ToString()
 {
 string output = String.Format("Title :{0} \n Author :{1} \n Publisher :{2} \n Price {3}", this.title, this.Author, this.publisher, this.price);
 return output;
 }
 public static List<Book> myList = new List<Book>();
 private static void changegenre()
 {
 string mygenre = "Fantasy";
 genre = mygenre;
 }
 static void Main(string[] args)
 {
 changegenre();
 myList.Add(new Book() { title = "Hobbit", Author = "J.R.R Tolkien", publisher = "U & G", price = 34, });
 myList.Add(new Book() { title = "Two Towers", Author = "J.R.R Tolkien", publisher = "U & G", price = 55 });
 myList.Add(new Book() { title = "Opowiesci z meekhanskiego pogranicza", Author = "Robert M. Wegner", publisher = "Powergraph", price = 10 });
 myList.Add(new Book() { title = "A Dance with Dragons", Author = "G.R.R Martin", publisher = "Voyager Books", price = 25 });
 Console.WindowWidth = 95;
 Console.BackgroundColor = ConsoleColor.DarkMagenta;
 Console.WriteLine(" --------Welcome to our bookstore!-------");
 do
 {
 Console.BackgroundColor = ConsoleColor.Black;
 Console.WriteLine("Please write: \r\n a = if you want to search book by author, \r\n p = if you want to search book by price, \r\n pub - if you want to search by publisher or \r\n t- if you want to search by title");
 string input = Console.ReadLine();
 switch (input)
 {
 case "a":
 searchbyauthor();
 break;
 case "p":
 searchbyprice();
 break;
 case "pub":
 searchbypublisher();
 break;
 case "t":
 searchbytitle();
 break;
 }
 Console.WriteLine("Do you want to search again?");
 } while (Console.ReadLine().ToUpper() == "YES");
 }
 private static void searchbytitle()
 {
 Console.WriteLine("Please, write book's title!");
 string input = Console.ReadLine();
 foreach (Book result in myList.Where(x => x.title == input).ToList())
 {
 Console.WriteLine(result);
 }
 }
 private static void searchbypublisher()
 {
 Console.WriteLine("Please, write publisher's name!");
 string input = Console.ReadLine();
 foreach (Book result in myList.Where(x => x.publisher == input).ToList())
 {
 Console.WriteLine(result);
 }
 }
 private static void searchbyprice()
 {
 Console.WriteLine("Please, give me the maximum book price");
 string input = Console.ReadLine();
 int maxprice = Convert.ToInt32(input);
 foreach (Book result in myList.Where(x => x.price <= maxprice).ToList())
 {
 Console.WriteLine(result);
 }
 }
 private static void searchbyauthor()
 {
 Console.WriteLine("Please, write author's name!");
 string input = Console.ReadLine();
 foreach (Book result in myList.Where(x => x.Author == input).ToList())
 {
 Console.WriteLine(result);
 }
 }
}

Answer 1

Always nice to see a student eager to learn.

Class Book

Your should try to always use an access modifier, so this should be

public class Book

Other issues:

• Public properties in C#, should begin with a capital letter.
• Price should be a Decimal (used for currency or monetary values) instead of a whole number.
• Genre should rightfully be a property to a Book, and not static to the entire class. Changing the genre then becomes as easy as updating the property.
• myList does not belong in the Book class. It should not be static and should be moved to Main.
• ToString() does not need to refer this.. Nor is the output needed. Instead just return the string.Format.

Search Methods

The names should use PascalCasing, example SearchByPublisher, SearchByPrice.

There is no reason for the trailing .ToList() inside each respective foreach. The Where has already returned an enumerable collection that you will iterate over.

For comparing strings, you have no regard for case. Thus "HOBBIT" and "hobbit" would not match "Hobbit". You may want to allow for ignoring the case. Another consideration would be on matching a partial string. Consider if the user enters "dragons" at the console that maybe you want to return "A Dance with Dragons".

Going Further

As a beginner, be sure you grasp what you see so far. Then go a step further and add a Books class that implements List<Book>. Now the search methods could be moved inside the Books class. In the name of Separation of Concerns, you don't want the searches to also write to the console. Instead, the search should be concerned only with searching. That would mean each respective search would return an IEnumerable, or really just the results of the Where( ... ).

• Pandaemonium: Thank you! I have learnt a lot from your post. I didn't even notice, how chaotic my code was - I cannot praise you enough for reading it so carefully. Hopefully, I did not cause you an eye cancer. Please, could you share some sources where I can read more about separation of concerns and architecture? Thank you once again for your tips!
• Rick Davin: docs.microsoft.com/en-us/dotnet/standard/design-guidelines/…
• Rick Davin: en.wikipedia.org/wiki/Single_responsibility_principle