Ocena kodu

Prosiłbym o ocenę kodu w c#

https://github.com/OrientMantis24/Project-Film-Library

Patrzcie na to jak na projekt początkującego bez doświadczenia :)

To tak na szybko:

• skoro korzystasz z bazy sql to zamiast pisać te wszystkie statyczne metody w pliku FilmDatabase mogłeś użyć jakiegoś ORMa, chociażby Entity Framework wtedy miałbyś właściwie załatwioną całą obsługę odczytu/zapisu do bazy bo widzę, że nie masz tam raczej dodatkowej logiki poza select/insert
• w klasie Films nie widzę powodu, żeby pisać pola prywatne i gety/sety do nich, same właściwości by w tym wypadku wystarczyły czyli by klasa wyglądała tak:

namespace FilmLibrary
{
    public class Films
    {
        public int FilmId { get; set; }

        public string FilmTitle { get; set; }

        public string FilmDescription { get; set; }

        public string FilmDirector {get; set; }

        public string FilmGenres { get; set; }
    }
} 

O wiele krócej i czytelniej, co nie?

• plik View.cs też można mocno uprościć i oddzielić logikę od wyświetlania tekstu w konsoli, ale w tym momencie nie mam gotowej podpowiedzi więc może ktoś inny się wypowie

Pozdrawiam,
Marek

Edit: No i zauważ, że patrząc na funkcję main widzi się tylko wywołanie GenerateMenu() co sugeruje, że aplikacja po prostu wyświetli menu (a nawet nie, tylko je gdzieś wygeneruje), a nie widać żadnego start/run/execute/choose option/itp. Więc nazwa metody jest myląca

FilmLibrary.FilmDatabase

• w EditFilm sklejasz stringa zamiast użyć Parameters jak w przypadku AddFilm
• catch (SqlException ex) { throw ex; } jest zbędne do tego gubisz StackTrace

FilmLibrary.View

• nie musisz tworzyć konstruktorów dla delegatów myFoo = new Program.Foo(View.AddAFilm); wystarczy myFoo = AddAFilm
• wywołanie myFoo(); równie dobrze mogło by być na końcu w metodzie "GenerateMenu()"
• ifa można by przerobić na switch albo zrobić z tego jakąś kolekcje.
• AddAFilm zbędne rozbicie deklaracji zmiennych od przypisania wartości
• zbędne wywołania ToString()
• zamiast używać 1,2,3 itp. używaj w takich przypadkach enum
• w metodzie ChooseAFilm nie potrzebnie podajesz wartość początkową zmiennej filmNumber skoro i tak nic z tym nie robisz a dwie linijki dalej pobierasz wartość z konsoli.