"Masz rację, tak samo jak Twoja opinia. Tylko że ja swoją podpieram doświadczeniem i ustalonymi w branży dobrymi praktykami. Zwolennikami tego są znacznie lepsi niż my specjaliści tacy jak X, Y, Z."
Argumenty w stylu, Ja swoją opinię podpieram doświadczeniem albo ustalonymi regułami i dobrymi praktykami, są dla mnie zawsze tak samo żałosne.
To pokaż mi cwaniaku jak zrefaktoryzujesz tą metodę.
Tak żeby było mniej ifów i mniej linijek.
Jezu aż oczy bolą. W życiu nie spotkałem osoby pracującej w branży, która napisała by kod aż tak niskiej jakości. Jest tak źle, że podejrzewam prowokację!
Jakbym dostał, współpracownika, który pisałby coś takiego, to omówiłbym z nim na stronie, wszystkie problemy tego kodu.
Jeśli taka osoba okazała by się odporna na argumentację i byłaby przekonana, że to jest super, to nie widziałbym wyjścia, tylko przedyskutować to ze zwierzchnikiem, bo taki pracownik nie rokuje. Może taka osoba zrealizuje parę zadań, ale za parę miesięcy trzeba będzie naprawiać bugi, po takim artyście poświęcając na to 2 razy więcej własnego czasu.
Każdy robi bugi, ale w tym wypadku poprawianie kodu byłoby to wyjątkowo bolesne.
Podstawowe problemy opisała katelx:
ten kod to tragedia, testy jeszcze gorsze ;)
- potrzebne sa ze 2 metody - jedna dajaca nowy rozmiar obrazka, druga robiaca krojenie
- przeskalowanie rozmiaru to krotka formulka, nie trzeba zadnych ifow i pincet zmiennych
- to rzutowanie do decimala jest bezsensowne i bezuzyteczne
- uzywanie mutowalnych pol (i to statycznych) to patola level 99
- ten wyjatek to taki se
nie chce mi sie juz wiecej wymieniac, w sumie to nie wiem o co chodzi z tym pytaniem, wiem ze nie chcialabym pracowac z kims kto pisze taki kod
A problemów jest dużo więcej z tym kodem.
Disclaimer: jestem bardziej C++/Swift/ObjC niż C# więc pewnie będą niedoróbki językowe, albo złe użycie bibliotek, ale generalnie chodzi o pokazanie, że da się zrobić lepiej. Ograniczając się jedynie do punktowania wszystkich kwiatków, autor odbierze(rał) to personalnie i nie zrozumie, że naprawdę coś jest źle.
Jest tak źle, że lepiej zacząć od zera, bo naprawdę nie ma sensu poprawiać iteracynie:
interaface ImageTransformator
{
Image transform(Image image);
}
class ThumbnailGenerator : ImageTransformator
{
private final System.Drawing.Size size;
public ThumbnailGenerator(System.Drawing.Size expectedSize)
{
size = expectedSize;
}
public override Image transform(Image image)
{
if (doNotHaveToResize(image.Size))
{
return image;
}
return scaleToSize(image, thumbnailSizeFrom(image.Size));
}
public static Image scaleToSize(Image image, System.Drawing.Size size)
{ // przeróbka z tego: https://stackoverflow.com/a/2001462/1387438 więc mogłem coś popsuć, nie testowałem
Bitmap bmPhoto = new Bitmap(size.Width, size.Height, image.PixelFormat);
bmPhoto.SetResolution(image.HorizontalResolution, image.VerticalResolution);
using(Graphics graphics = Graphics.FromImage(bmPhoto)) {
graphics.Clear(Color.Red);
graphics.InterpolationMode = InterpolationMode.HighQualityBicubic;
graphics.DrawImage(imgPhoto,
new Rectangle(0, 0, size.Width, size.destHeight),
new Rectangle(0, 0, image.Width, image.Height),
GraphicsUnit.Pixel);
}
return bmPhoto;
}
private bool doNotHaveToResize(System.Drawing.Size actualSize)
{
return actualSize.Width <= size.Width && actualSize.Height <= size.Height
}
private Size thumbnailSizeFrom(Size actualSize)
{
if (actualSize.Width / (double) size.Width >= actualSize.Height / (double) size.Height)
return new Size(size.Width, size.Width * actualSize.Height / (double) actualSize.Width);
return new Size(size.Height * actualSize.Width / (double) actualSize.Height, size.Height);
}
}
class ImagesConverter
{
private ImageTransformator transformation;
MapImages(ImageTransformator transform)
{
transformation = transform;
}
List<Image> transform(List<Image> images)
{
return images.Transform(image => transformation.transform(image));
}
void TransformFiles(String fromDir, String toDir, String prefix)
{
Directory.EnumerateFiles(fromDir, "*.jpg")
.Transform(file => (Image.FromFile(file), file.Substring(fromDir.Length + 1)))
.Transform((image, name) => (transformation.transform(image), prefix + name))
.Where((image, name) => image != null)
.Do((image, name) => image.SaveTo(toDir + name);
}
// inne api operowania na zbiorze obrazów
}
Nazwy klas mi się nie podobają, w takich wypadkach zostawiam do poprawy podczas code review z komentarzem prowokującym do sugesti innej nazwy.
Nie chce mi się pisać części odpowiedzialnej za zapis i odczyt z plików, ale jak widać można to ładnie odizolować od części generującej miniatury od iterującej po obrazach zapisanych w różnych formatach.
Jeden algorytm może obsługiwać nie tylko generowanie miniatur, ale też zmianę balansu bieli, kowersje na obraz czarno biały itp itd.
Tak samo klasa do generowania miniatur może być użyta winnym kontekście.
Pisanie testów do tego też nie jest problemem.
Widać też, że przeczytanie i poprawienie tego kodu jest dużo łatwiejsze niż oryginał.