Bleh, stronka kiczowata niczym z przełomu XX i XXI, zainteresuj się Bootstrapem czy innymi frameworkami.
Odnośnie kodu:
HWinfo.cs
OSInfo() - czy aby na pewno potrzebujesz tu zmiennej system?
RAMSize() - dlaczego to zwraca ciąg znaków, a nie liczbę?
InstalledDiscs() - dlaczego to zakłada, że separatorem jest znak nowej linii? Powinno zwracać raczej tablicę.
Processor() - czy aby na pewno potrzebujesz tu zmiennej cpuname? I dlaczego domyślna wartość to "null"?
GraphicCardName() - to samo cd. InstalledDiscs, czyli z serii "a co jeśli chciałbym mieć to po przecinku?"
GraphicCardMemory() - dlaczego to nie zwraca inta?
DrivesInfoPL() - obrzydliwie brzydko sformatowana metoda.
No i generalnie znowu wracamy do łamania zasady DRY - nie widzisz, że strasznie często tam się powtarza kod w stylu:
public string FajneDane()
{
string system = "unidentified";
try
{
ManagementObjectSearcher searcher = new ManagementObjectSearcher("root\\CIMV2", "SELECT * FROM Win32_FajneDane");
foreach (ManagementObject queryObj in searcher.Get())
{
return queryObj["FajneDane"].ToString();
}
}
catch (Exception)
{
return "Unknown error";
}
return system;
}
Plus lepsza jest nazwa metody GetWindowsDirectory (lub GetWindowsPath) ponad jakieś WindowsDirectory, to samo odnośnie całej reszty - GetCpuCaption ponad CPUCaption, GetCpuL2CacheSize zamiast CPUL2Cache (...).
Idziemy dalej.
Tak w sumie znikąd wziął się katalog MainWindowClass oraz podkatalog code, do którego wrzucone zostały jakieś jeszcze dziwniejsze pliki, na pierwszy rzut oka niepowiązane w żaden sposób z czymkolwiek jest MainWindowClass.
ClearFunction.cs
- nie czaję zupełnie, dlaczego ten plik rozszerza klasę
MainWindow - przecież ClearTemp nie ma nic wspólnego z klasą zarządzającą głównym oknem.
- na plus, że trochę zmniejszyłeś moje ówczesne palpitacje serca wywoływane patrzeniem na podwójne metody.
- na minus, że są jakieś dziwne warunki w stylu
if (Directory.Exists(Path.GetTempPath()) == true) - tak jak gdyby if (Directory.Exists(Path.GetTempPath())) nie było już wystarczającą kobyłą, podobnie odnośnie tych wszystkich if (result == true), skoro samo if (result) wystarczy.
- nadal nie zapoznałeś się z blokiem
finally, przez co masz w kodzie takie kwiatki jak dodawanie rozmiaru pliku w try i odejmowanie go w catch, gdyby coś po drodze nie wyszło, zamiast po prostu dodawania go w finally, gdy wszystko poszło ok.
-
sizeallfilestemp - taksięniepiszenazwzmiennych. totalBytesProcessed, totalFilesProcessed brzmią ładniej.
-
IsClean? Co to za nazwa parametru? "czy czyste"? Lepiej nazwij go "czy wyczyścić", np.: ...(bool doClean).
- dlaczego każda metoda ma osobny zestaw
sizeallfile...? Mógłbyś z tego zrobić pola klasy (tylko dwa! totalBytesProcessed i totalFilesProcessed, bez rozdrabiania się), opakować w coś ładnego i mieć wycięty powtarzający się kawałek kodu do odświeżania GUI.
- nadal występuje bardzo dużo DRY. Nie widzisz tego, ile kodu powtarzasz? Gdybyś miał napisać to bez korzystania z Ctrl+C/Ctrl+V to nie byłoby już tak różowo, prawda?
- mieszasz nazewnictwo, raz jest
ClearCacheThumbnails, potem Clearreporterrors - raz jest sizeallfileLD, a następnie z kolei FileTemp.
ClearFirefoxCache() - //can always delete, because ff is closed to dosyć optymistyczne założenie, zwłaszcza że usunięcie pliku to nie jest nic dziejącego się "ot tak" i po drodze całkiem sporo może nawalić, nawet jeśli Firefox jest zamknięty.
Error() - nie stawiaj spacji przed znakami interpunkcyjnymi (Czy chcesz zapisać kod błędu w celu przekazania go do analizy ?)
ContextMenu.cs
- znowu jakieś przyrównywanie do wartości logicznych zamiast podejścia sensownego - nie pisz
if (Check_OS.IsChecked == false), tylko if (!Check_OS.IsChecked)
- raz jeszcze masa powtórzonego kodu. Pokażę Ci sztuczkę.
Przed:
private void Explorer_Click(object sender, RoutedEventArgs e)
{
if (Check_Explorer.IsChecked == false)
{
Check_Explorer.IsChecked = true;
//
CheckBox_LatestDocuments.IsChecked = true;
CheckBox_CacheThumbnails.IsChecked = true;
}
else
{
Check_Explorer.IsChecked = false;
//
CheckBox_LatestDocuments.IsChecked = false;
CheckBox_CacheThumbnails.IsChecked = false;
}
}
Po:
private void Explorer_Click(object sender, RoutedEventArgs e)
{
bool isChecked = !Check_Explorer.IsChecked;
Check_Explorer.IsChecked = isChecked;
CheckBox_LatestDocuments.IsChecked = isChecked;
CheckBox_CacheThumbnails.IsChecked = isChecked;
}
InstalledPrograms.cs
WhatInstalled() omujborze, tony powtarzania tego samego w kółko i w kółko, pomyśl jak to skrócić. Pomijając oczywisty fakt, że WhatInstalled to nawet nie jest poprawne angielskie sformułowanie. WhatIsInstalled jak już, a najlepiej DetectApplications. Kolejny raz także rozszerzasz MainWindow - powiedz mi proszę, dlaczego i po co? Przecież to byłoby perfekt na osobną klasę statyczną.
Language.cs
LoadLanguageCore() - oh god why. Gdybyś miał 100000 kontrolek to też wczytywałbyś jedna po jednej, linia po linii? Zapoznaj się z plikami ini bądź xml bądź po bądź bazą SQLite (...), łapiesz schemat? I po zapoznaniu zrób tak, aby ta metoda była niezależna od liczby kontrolek, tak abyś nigdy więcej nie musiał jej zmieniać, lecz bez problemu mógł dodawać i rozszerzać program o następne buttony, menu etc.
WhatLanguage() - nah, nah, klasa odpowiedzialna za język nie powinna być odpowiedzialna za parsowanie konfiguracji. Od tego powinna być osobna klasa z metodą GetLanguageId() (widzisz także różnicę w czytelności nazw?).
ChooseLanguage() - to nie ma nic wspólnego z obsługą języka, wydziel to gdzie indziej.
RunPrograms.cs
ProcessFireFox - FireFox? procesyall? ffrun? Co to w ogóle za nazwy? Poza tym nazwa tej metody nic nie mówi, IsFirefoxLaunched brzmi ładniej i dokładnie opisuje, co ona zwraca. No i ten, ehm, dlaczego zwraca inta, a nie boola?
To samo odnośnie reszty tych metod, i podobnie jak wyżej - w jakim celu to rozszerza klasę MainWindow?
SystemInfo.cs
OSandHWInfo - kolejna wykwintna nazwa metody, formatowanie jeszcze lepsze.
SaveInfoToFile - jeden komentarz po polsku, drugi po angielsku, potem znowu po polsku, czyżby Anglicy wbili nam się do okopu?
Wychodzimy z katalogu, wracamy do głównego.
WhatNETVersion.cs
NETVersion - nazwa metody sugeruje, że zwracasz wersję .NET Frameworka, tymczasem metoda zwraca bool... hm...
CheckFor45DotVersion - jak już bierzesz kod ze StackOverflow to chociaż zalinkuj do niego w komentarzu lub zmień nazwę metody...
MainWindow.xaml.cs
MainWindow - tutaj pojawia się wywołanie jakiegoś magicznego configweryf, którego nie zauważyłem przedtem. Też intrygująca nazwa metody. Wprowadza wręcz taką nutkę erotyki.
A może to mi się już w głowie coś miesza.
Analizuj_Click - god bless his soul, powiedz mi proszę, że widzisz, jak bardzo ta metoda jest upośledzona :P
Hm, no i to by było na tyle.
Wyszło tego znacznie więcej, niżbym pomyślał, co jednak powinno Ci dać do zrozumienia, że warto poznać podstawy, zanim zabierzesz się na poważnie za coś "większego".
Poprawianie tego kodu nie miałoby najmniejszego sensu - wyrzuć go i zacznij pisać od zera, uwzględniając wszystkie moje powyższe słowa, a wyjdziesz na tym lepiej.
No i powodzenia ;)
Edit: byłoby także miło, gdybyś wykazał się jakąś oryginalnością, a nie kopiował wygląd z CCleanera.
Edit2: plus mail założony w domenie aplikacji jest raczej średnim pomysłem, chyba że jest to bezpośrednio [email protected].
Edit3: plus kompletnie nie rozumiem, w jakim celu na stronie jest przycisk Archiwum aktualności, skoro jest to podstrona aktualnie nie dostępna (btw, nie z przymiotnikami łącznie).
