I've written a lot of bad code, seen a lot, but attached is the most recent 'worst' I've seen.
What makes the situation worse/funny is:
1. The developer's code comment.
2. Check-in passed a code review.
3. The 'legacy code' was written last week.

What the actual FUCK?

Why did this pass review...why is this happening...why do I feel like this is in production

@PaperTrail
I'm assuming that InvoiceSearch is a DTO of sorts coming in from a HTTP request.

@Devintrix I suspect the code review was part of a 50+file check-in and the reviewer spent two minutes looking at the files and said "Screw it ...Great job!"

Uh a loop evaluating each attribute with a closure and breaking on something falsy would do? :p

@GinjaNinja It is/are the property values on the ViewModel to enable/disable the search button.

Wrapping the whole thing in an if would make it faster.

@PaperTrail haha so me when I am swamped. Shitty practice I see now I have to beore thorough

@industry-monkey Totally do not understand how that works. Do explain please

At least the comment isn't lying.

Good lord...

So how would u fix it?

Seems actually this maybe the cleanest way.

Maybe not most efficient but it explicitly lists out each case

@billgates There are probably several ways to skin that cat. *I* would create a user-control that encapsulates the desired searching criteria and return whether or not it (the control) contains valid search parameters. Ex. InvoiceNumber implements a ISearchable interface and would return TextBox1.Text <> "" (pseudo code, but you get the idea). A controller would/could iterate through all the specific user-controls (if control is ISearchable ...) and set/enable/disable whatever bound search UI control (ex. search button). Each control would be responsible for the desired behavior and best of all, unit testable.
A little more component design upfront, but adding new UI controls later becomes easy to integrate and debug.

@PaperTrail but wait that's be GUI logic? But in backend still should verify before storing the data.

It's like JS validation, still should check on backend in case user bypasses it in the browser.

@billgates Sorry, I should have added this is a monolith C#+WPF application. Some devs that work here are finally realizing putting GUI logic in the GUI saves them a lot of time and trouble. Trying to 'hide' user interaction logic in ViewModels has caused them nothing but pain and grief.
My laziness forces me to find easier solutions than adhere to a purest viewpoint of the MVVM pattern. If I had enough space, there is somewhat long rant story behind that.

@PaperTrail But what about those compound cases? I guess for MVVM there should be a data validation logic You could put into the Properties that you bind to or when you hit Submit someway to return a list of errors and the UI fields to Mark red.

If this were a webpage, I'd prolly return like a list of errors and show them as bullet points.

Be thankful that comment didn't say: better be safe than sorry!

@tisaconundrum processing pipeline branching strategies https://web.cs.iastate.edu/~prabhu/...

The real issue I have is that no one has proposed a better solution. It's all about how shit the code is... and yes, I'm guilty of that too!

But the junior dev here and myself looked at this, swore a few times and tried to think of a better way.

So, last night I looked at it again and the solution below sprung to mind. Clearly this is C# code, so why not leverage on what tools you have at your disposal and also what the language has to offer. Maybe it's just a clever way of hiding that mess by what I've done here. But hey, with this solution it's easier to test and possibly even reusable, cause I'm sure this isn't the only place.

Use it, don't use it...

@GinjaNinja what about the && ones?

And I did propose a solution, you may have implemented it.

But that (Min, Max) Amount... I've never seen syntax like that.... Is that the new version of NET?

@billgates
You did make mention of Properties, which is what I also initially thought of as well. But this solution just feels like we would be moving the problem (read ball-O-mud) elsewhere. And also, this doesn't need a list of any kind as feedback to the user. Displaying lists for validation errors makes for shit UX anyways!

With the attributes, we would be leveraging the language and that would in turn make for a cleaner, more readable solution.

As for the &&'s, well those I've solved by making use of Tuples (the new C# 7 Value Tuple feature) for properties like StartDate/EndDate and MinAmount/MaxAmount along with a RequiredRange attribute.

As for the other properties further down, they seem to be cut off and so I can't quite make out the && logic there, but I'm pretty sure that's not too tricky to apply some kind of validation attributes to either!

@GinjaNinja but you used Properties in your solution 😀

Also language upgrades are expensive in enterprise applications... especially if you need to also upgrade the IDE every time as well. Lucky my bro is in college, I got VS 2013, 15, 17 Enterprise from his DreamSpark access 😄

Our company mostly uses VS2010/NET 3.5 unless your working on a new app.

@billgates
Sure I used Properties I my solution 😉 They're dumb properties is terms of only having Get and Set and not fiddling with backing fields and internal fields.

The real magic lies in the attributes and reflection, and what that they allows us to do! 😎

As for it being "expensive" for a language upgrade... that is unfortunate and I'm sad to say that it's also tell tale sign of a very sick way of working in the software business if you can't be on the latest and greatest version of something, especially a language!

@GinjaNinja

Getters and Setters there are ways, just refactor the common logic.

Well just look at AngularJS and Python.

There major breaking changes between versions so now we before have 2 languages each: AngularJS, Angular, Python 2, Python 3

@GinjaNinja Because I use MVVM bindings need to RaisePropertyChanged so I made helper functions.

Now it's just copy-paste, change the name and return type.

The actual data is stored in a Dictionary<string,object> in the abstract base class.

@billgates
Implementing INotifyPropertyChanged is an absolute ball ache! 😖

Here are articles for two, slightly "cleaner", approaches...

Uses .NET 4.5, so that might not be viable...
https://danrigby.com/2012/04/...

Auto wire attribute, created to help alleviate WPF ball ache, which I think is a very elegant solution indeed!
https://codeproject.com/Articles/...

Yes, the first is what I did after I finely switched to 4.5. Before have to explicitly pass in the property name.

#2 one I stopped reading after seeing the huge chunk of code.

That actually gave me brain damage

It's almost like it's a solve for some other monstrous gaping hole in the app. Like....a complete lack of referential integrity or something. Who the fuck knows?

It's the the thing that should not be.