I am leaving my first job because I always worked alone on projects from my day one, which was both useful but also taught me some bad practices some of which I managed to realize but many others are still there. Not being able to learn from colleagues, constantly being pushed with deadlines because it's a really small company with 3 spaghetti code developers. In certain parts of the projects I focus on architecture, I read a lot, trying methodologies and different approaches but once deadlines and grumpy management comes across, my code quality drops tremendously.
One of the things that bother me is that in nearly every project I often end-up writing things like the examples below, filling classes and having a lot of logic in the direct assigning of properties, teritary operators, LINQ and whatnot knowing this is definitely bad but never bothering to look for a solution.
Filling a hotel information class that comes from a SOAP service.
var offersToBind = filteredOffers.Select(h => new HotelInformation { OtherOffersCount = h.AllOffers.Count, PriceType = h.LowestOffer.Rates == null ? "Редовна цена" : h.LowestOffer.Rates[0].SpoInfos.Any() ? GetSpoTypeName(h.LowestOffer.Rates[0].SpoInfos[0].SpoType) : "Редовна цена", HotelImage = h.HotelInfo.FirstImage() != null ? GetHotelPicture(h.HotelInfo.FirstImage()) : "http://loremflickr.com/320/240/dog", HotelName = h.HotelInfo.HotelDetails.FirstOrDefault(hd => hd.LanguageId == LangId).Name, HotelPrice = (h.LowestOffer.TotalPrice * 1.95583).ToString("0.00") + " лв.", HotelBoard = h.LowestOffer.BoardName, HotelResort = resorts.FirstOrDefault(r => r.SejourId == h.HotelInfo.ResortID)?.ResortDetails.FirstOrDefault(r => r.LanguageId == LangId)?.ResortName, ExtrasPrice = (h.LowestOffer.ExtraPrice * 1.95583).ToString("0.00"), RoomName = h.HotelInfo.HotelRooms .Where(r => r.SejourRoomType == h.LowestOffer.RoomType && r.SejourRoomTypeName == h.LowestOffer.RoomTypeName && r.SejourRoomName == h.LowestOffer.RoomName && r.SejourRoom == h.LowestOffer.Room && r.LanguageId == LangId) .Select(r => r.Name) .DefaultIfEmpty("[Missing room information]") .First(), DateFrom = periodFrom.ToString("dd.MM"), DateTo = periodTo.ToString("dd.MM"), Nights = (periodTo - periodFrom).Days, Identificator = Helper.GenerateRandomString(8) }).ToList();
Another example of the same issue but this time working locally in a similar manner and passing the select result to a repeater.
promotions.Select(p => new { PromotionDateRange = p.DateStart.HasValue & p.Date.HasValue ? p.DateStart.Value.ToString("dd/MM/yyyy") + " - " + p.Date.Value.ToString("dd/MM/yyyy") : string.Empty, PromotionImage = p.Media.Any() ? ConfigurationManager.AppSettings["ProductPictures"] + p.Media.First().FileName : string.Empty, PromotionPoints = p.Points, PromotionName = p.Name, PromotionContent = p.Contents, PromotionPrice = p.Price.HasValue & p.Price.Value != 0 ? p.Price.Value.ToString("G") + Currency : string.Empty, PromotionOldPrice = p.OldPrice.HasValue & p.OldPrice.Value != 0 ? p.OldPrice.Value.ToString("G") + Currency : string.Empty, PromotionPriceVisibility = (p.Price.HasValue & p.Price != 0) && (p.OldPrice.HasValue & p.OldPrice != 0) ? string.Empty : "display: none", DiscountPercent = p.Discount.HasValue ? p.Discount.Value.ToString("#") + "%" : string.Empty, DiscountAmount = p.OldPrice.HasValue & p.OldPrice.Value != 0 & p.Price.HasValue & p.Price != 0 ? $"({(p.OldPrice - p.Price).Value.ToString("G") + Currency})" : string.Empty, PromotionDiscountVisibility = (p.Discount.HasValue & p.Discount.Value != 0) ? string.Empty : "display: none;", PromotionLink = $"/{LangCode}/product/{p.ProductID}" });
I know this logic has to happen or be written somewhere, even if it's in a different manner, but I am not sure how to approach it. I had some ideas, but my ideas are not always guaranteed to be the common approach the software community is using. Which is why I decided this is the best time to start asking isolated questions about isolated issues.
I was thinking of some object initializers and moving the logic there, but I would still eventually write that kind of logic and checks one way or another and it's really hard to read. There are other bad apples here, the smaller type but they aren't my focus currently.
promotions
? Where to they come from? What's aHotelInformation
? Where are thefilteredOffers
and why do you needoffersToBind
? Tell us more about your code, show us more of it, and less of your bio. \$\endgroup\$