ObservableCollection is a Leaky Abstraction

And so is BindingList. Or, rather, exposing them through your controller’s interface is a Leaky Abstraction.

Anyone who’s done any Windows desktop development has eventually come across ObservableCollection and BindingList. How often have you written a controller (ViewModel or Presenter, doesn’t matter what you call it, they’re all some variant of the Controller in MVC) that looks like this.

    public class LeakyWpfController : INotifyPropertyChanged
    {
        public ObservableCollection<Person> People { get; } = new ObservableCollection<Person>();

        public event PropertyChangedEventHandler PropertyChanged;
    }

Or this

    public class LeakyWinformsController
    {
        public BindingList<Person> People { get; } = new BindingList<Person>();
    }

What’s wrong with this code?! We need to expose a collection type that the framework understands for the databinding to work!

No. We don’t. Both Winforms and WPF don’t care what you expose. They only care that the backing field is of the proper type. All you need to expose in your public interface is an ICollection, in both cases. Databinding to these two controllers works perfectly well.

    public class WpfController : INotifyPropertyChanged
    {
        public ICollection<Person> People { get; } = new ObservableCollection<Person>();

        public event PropertyChangedEventHandler PropertyChanged;
    }

    public class WinformsController
    {
        public ICollection<Person> People { get; } = new BindingList<Person>();
    }

Ok, so big deal. It’s not like I’m ever going to use my ViewModel or Presenter for anything but the specific View they were built for…

Are you so absolutely sure of this? If you’re certain that you’ll never want to use the same presentation logic in more than one window or application, then sure. You’re probably right. Leaking this abstraction doesn’t hurt, but what if we wanted to reuse logic from our old Winforms application in our new WPF app? Or what if we wanted to set ourselves up to migrate away from that old Winforms app into a new WPF application? What do we do when WPF and ObservableCollection aren’t the newest, greatest thing anymore and we’d like to migrate to that newest framework? Wouldn’t it be nice to be able to do so with minimal effort?

Our two controllers are at this point so similar, we can combine them into one. (Thanks to the fact that WPF supports BindingList as a backing field.)


    public class Controller : INotifyPropertyChanged, IController
    {
        public ICollection<Person> People { get; } = new BindingList<Person>();
        public ICommand AddItemCommand = new AddItemCommand(this);

        public event PropertyChangedEventHandler PropertyChanged;
    }

I only recommend using BindingList as a backing field if your Controller (view model/presenter) needs to work in a Winforms environment though. There are some performance issues with BindingList. If you only need to target WPF or UWP, then ObservableCollection is the right way to go. Either way, there’s no reason to expose anything more than an ICollection to the outside world.

, , , , , , ,

Leave a comment

IT as Couples Counseling

Do you ever feel like a teenager passing messages between your separated parents?

Joe in receiving puts in a ticket asking you to look into an issue. You take a look at it and realize that it’s because Barb in purchasing failed to check a box. So, you email Barb and ask her to correct it. Barb corrects it and you go back to Joe to see if his issue has been resolved.

Joe says, “Yup. Looks good now. Thanks. This is the 3rd time this week those idiots over in purchasing screwed up.”

“What do you mean this was the third time this week?”, you say.

“Well, I got tired of complaining about it, so it was easier to just put a ticket in. Those asshats know the process, they just don’t do it and it causes us to get behind. I don’t have time to go hunt them down every time they screw the pooch.” Joe laments.

You want to reply, “So you wasted my time on an issue you could have fixed yourself?!”, but instead you just sigh and walk back to your desk, dejected. There was no reason for IT to even be involved in this. Joe could have just called Barb, politely asked for her to fix it, and moved on with his day. He didn’t because he’s angry at them and it was easier to have IT address it. Why not? IT fixes all the problems, don’t they?

We could easily just continue to just deal with the problem as it interrupts us, but that doesn’t address the issue at all. It’s just putting our heads in the sand and saying “Not my job.” If we want to stop playing messenger, then we’re the ones who need to take action. Once a feud between two departments in an organization begins, it’s extremely unlikely that it’s going to end without outside intervention. Like it or not, IT is a neutral party that both sides of the problem trust and it’s up to us to fix it.

Read the rest of this entry »

Leave a comment

The Rocky Road to Agility

A few months ago, I wrote about the beginning of our journey on the path to agility. Since then, we’ve made some major strides and also had some serious set backs.

Our process was getting better, but we continued to struggle with the legacy code base. I was writing tests and refactoring the code base, but was still the only one doing so. The team was seeing the benefits I was getting out of it, but simply didn’t know how to do it themselves.

Read the rest of this entry »

, , , , ,

Leave a comment

Be Careful with ToList()

I recently came across some code during a review that seemed perfectly fine at first glance, but upon further inspection, had potential to perform terribly. Take a look. What’s wrong with this code?

using (var context = new DbContext())
{
context.SomeTable.Where(t => t.Id == someId)
.GroupBy(t => t.Category)
.Select(tg => new { tg.Category, Profit = tg.Sum(p => p.Profit) })
.ToList()
.ForEach(SaveToDb);
}

Do you see it? It’s easy to miss.

Calling ToList() on the Enumerable materializes the query, iterating over the Enumerable in order to generate the List. Then, just a moment later, we iterate over the List. We’ve potentially doubled the run time of this method. In most cases, likely not a big deal, but what if the List contains thousands of items? We’re doing more work than necessary here.

Fixing this is a simple matter of storing the IEnumerable in a variable and using the traditional foreach syntax instead of the ForEach extension method.

using (var context = new DbContext())
{
var query = context.SomeTable.Where(t => t.Id == someId)
.GroupBy(t => t.Category)
.Select(tg => new { tg.Category, Profit = tg.Sum(p => p.Profit) });

foreach(var item in query)
{
SaveToDb(item);
}
}

It just goes to show that you really have to be paying attention when reviewing (or writing!) code. Some issues can be terribly difficult to spot at a glance.

, , , , , ,

Leave a comment

Stay true to the spirit of Agile

Agile isn’t just a methodology or a big buzzword but it is a mindset which encompasses a set of values and principles which are light-weight and if implemented correctly Agile can pave the way for creation of great products. Organizations which stay true to the agile principles not only promote flexibility and openness but are also fun to work

https://agileabdulblog.wordpress.com/2016/02/06/stay-true-to-the-spirit-of-agile/

,

Leave a comment

Drinking the Agile KoolAid

If you follow me on Twitter, you’ve probably noticed that I’ve been talking and reading a lot about Agile lately. I’ve drunk the koolaid; I’m a believer. If you actually know me though, you know that this wasn’t always the case. I used to mock all of the zealots and their precious Agile methodologies. So, what changed? There has to be a reason for this. There is and it’s also why I’ve not been as involved in Rubberduck and this blog lately. 

I used to work on a small team for a large retail grocer. We were agile (little “a”). We had to be. In the retail grocery world, if you’re not, you’re sunk; you’re done, go home. There’s a constant flow of products being introduced and retired, new sales and marketing strategies, etc. This means that not a day goes by without someone needing more from the system you’ve built. You have to be able to react quickly and accurately to the business’ needs. 

Like I said, we mocked the Agile folk, with their daily stand ups, planning meetings, etc. because we were truly agile. We worked in small batches, kept our code clean, tested, and flexible, refactored as needed, had direct access to our users (and took advantage of it), deployed early and often. Man, we were good and we knew it, even if we were working with ancient tech, we consistently delivered value to the business on a regular basis.  
Fast forward a year, and I’m no longer working for that grocer. I started working for a small manufacturing company closer to home. No tests, no documentation, no real deployment process, and a ton of legacy code where the previous idea of reusing code was Ctrl+C, Ctrl+V. Some of this stuff is a real nightmare, but I’ll just refer you to The Daily WTF to get a feel for what the existing code base was like. But hey, I get to work with .Net instead of VB6!

At the interview, they told me they were going through an Agile transition. I was pretty excited about that, so needless to say I was a little disappointed at the state of things when I arrived. The only “Agile” thing in sight was a kanban board without any WIP limits and a daily stand up that was really just management’s daily status update. There was lots of talk about wanting and needing to be more agile though. That was a good start. 

So I dove in. I created a shared folder in our google drive and started putting whatever notes I needed to make for myself to implement a feature in it. I started making small refactorings to the code, mostly renaming variables and extracting methods. I was just trying to make sense of things. 

Pretty soon, my team mates were putting their own notes in the shared drive too, but every time I released a new feature, something broke. “This can’t continue.”, I thought to myself. “I’ve got to get this under test.” 

So I asked our OPs team to set up a VM for testing and tried. I did manage to get the code (in the area I was working in) under test before adding a new feature. The number of critical bugs released to production had significantly dropped, but the work was slow. It was taking far too long to get anything done and released. It was then that I remembered some things I had read in “Working Effectively with Legacy Code” by Micheal Feathers. 

Good is the enemy of better. 

I didn’t need to get all of the code under test, but I did need to make sure that I wasn’t breaking any existing code. I stopped trying to wrap tests around the one layer legacy system (Everything was stuffed in a webforms code behind). Instead, I created a new class library in the solution. Any new or changed behavior went into a new class in that library and got a full suite of tests wrapped around it. I didn’t change the legacy code beyond swapping out or adding a call to the new, tested, code. Now I knew that the whole system worked because new code was tested and the old code hadn’t really changed. 

The number of bugs released had dropped further, but bugs will always happen. Fixing them had become easier too, because I could add a new (failing) test case and it would point me right to the lines of code that needed to be changed. Deploying those changes however, was still far too manual, risky, and time consuming. 

I set about migrating our ASP.Net web sites to web apps so that I could leverage config transformations and deployment packages. This was a huge boon to my productivity and worth every second it took to do. I no longer had to manually check the diffs to see which files I needed to copy/paste deploy. All I needed to do was build a new version in RELEASE configuration and deploy. Within an hour of finding a bug in prod, I could deploy a fix for it. I was starting to gain the trust of my users. Yes, we were still going to have occasional regression issues, but we could fix them quickly. 

The code I worked on was getting better, but that was just my code. The rest of the team was still mucking through the legacy system with their cowboy boots on. Everyone understood the value of tests, good designs, and being agile, but just didn’t know how to do it. Unfortunately, I didn’t know, and perhaps still don’t, know how to teach what I knew about being agile. Our process was still a mess and I didn’t know how to fix it, or communicate my knowledge. I needed to find a way to do both. 

The team knew a bit about big A Agile, so I figured my best hope was to communicate in those terms, but I was loathe to do it. So much of the talk about Agile these days focuses on the processes and tools that it’s sickening. 

Individuals and interactions over processes and tools

The Agile Manifesto

The other thing I always hated about “Agile” was the time boxing. I knew from years of experience that time boxing the development of systems for internal use was a death march. It may work fine for new or external facing development, but working in sprints with hard deadlines for internal systems will just never work. The business needs this functionality, or this bug fixed now, not a week and a half from now. I needed a way to keep what agility we had, but also needed a way to move forward. 

I started reading. A lot. I began with the manifesto and that’s where my first glimmer of hope was. No where was there any mention of any specific process. No where did it say that you must work in two week sprints or hold a daily meeting. It simply said that there are ways to increase your agility, and that there is no one true way to do it. There’s hope, but what is this? Everything I’d ever heard about Agile involved silly things like time boxing and mob programming… I kept reading. 

It’s turns out that everything I knew about Agile was really everything I knew about Scrum. There were other Agile methodologies out there that I knew nothing about. “Surely one of these will work for us.” I kept researching. 

Then I found it! I found kanban. No time boxes, limited context switching via WIP limits, a prioritized backlog that still let us re-prioritize on the fly. This is what I had been doing all along without ever knowing it. I already had the process, but now I had the words. “This will work.”

I immediately shared what I had learned about kanban with management and explained what was wrong with our current board. I got permission to revamp it. I got the team together and explained what I wanted to do. We groomed the backlog and worked with the stakeholders to prioritize it. Things were starting to look up. 

At least until I received some very vocal feedback and concern about our “new” process. 

“WIP limits! But what if something is broken and needs fixed now?!  You’re telling me I’ve gotta finish these other things first?!”

“Well… Yes and no. By all means, if something is broken, fix it now. Don’t make the business wait.”

“But then the WIP limits are worthless!”

I had a problem. I knew that the WIP limits were important in order to reduce cognitive context switching. We couldn’t afford to lose them, but we were still living with a shody system. Things broke at least several times a week. At least I had the weekend to think about it. 

I spent my weekend doing more research on kanban. I watched several videos and read several articles before I found my answer. Swimlanes. What we needed was an “urgent” swim lane. Whenever something was broken, or a VIP made a request, we could use this swim lane. Anything in that lane was the priority. It would allow us to “safely” break our WIP limits and, perhaps more importantly, keep track of the number of urgent tasks the team was dealing with. 

I went back to work on Monday, solution in hand. The team reacted well, and we implemented immediately. The goal was to track the number of urgent tasks we faced and reduce them over time. Fantastic! We’re making progress. 

At least until the next complaint rose. 

“So, if we just keep pulling the highest priority task, how am I supposed to tell the business when something will be done?

“How do you estimate that today?”

“… based on my workload and priorities.”

“Okay, so keep doing that.”

“But, if the priorities can change, how can I tell them?!”

“Don’t our priorities already change fairly often?”

“Yeah, but…”

It was evident to me pretty quickly that this wasn’t going anywhere. I needed a way to make this team member feel comfortable doing the same thing he’d always done, but with a new name. I had nothing. Well, I did, but I really didn’t want to go that route. I really, really, didn’t even want to think it. Time boxing.

Now, I knew the effects that time boxing has on quality. As you get closer to the end of a sprint, people will cut corners in order to “just get it done”. This was absolutely the last thing I wanted to happen. I couldn’t have this happen. The department had traded quality for speed for years. Which in the long run is exactly what had slowed development to a crawl. “No. There has to be another way…”

I discretely pulled our most senior dev aside and explained the situation, both my co-workers concerns and mine. He had some experience with “Scrum light” and I valued his input. After some discussion, he convinced me that there was a bit of merit to having a two week iteration. (We use the iteration because it doesn’t have the same psychological effect as “sprint”. We didn’t want anyone rushing anything to meet an artificial deadline!) It would allow us to better track how much work we actually got done and improve our estimations over time, but he agreed that it would put quality at risk. We decided on a compromise. We have the team estimate what we could get done in two weeks, but with the explicit understanding that those estimates would be awful (we had no historic data after all), and no one was to sacrifice quality in order to meet an artificial deadline. If an item didn’t get done, it simply got pushed to the next iteration as the highest priority item in the queue. It was still kanban and it wasn’t a real timebox. Grudgingly, I agreed that this was a win and rounded up the rest of the team to discuss the idea. 

To my surprise, this has worked very well. To the best of my knowledge, no one has felt pressured to get something done just because the iteration is over. As an added benefit, we had a built in time to hold a retrospective. They’ve been entirely invaluable to our team. It’s a chance for us to reevaluate whether or not our process is working frequently, as a team. 

So, what we ended up with is some bastardized hybrid of Scrum & Kanban. I’m sure there are many Agile zealots out there who will tell me that we’re doing it wrong, but I don’t care. What we’re doing works for us. We’re a better team with a better process than we’ve ever had before. We deliver more value, more frequently than before. We’re more agile and that’s more important than being “Agile” in my book. After all, no where in the manifesto does it say “Here’s the one true Agile way.”, it only points towards what some of those ways may be. It’s something that I think we’d all be wise to remember. I hated the idea of Scrum, but was able to see value in some of its practices, so we borrowed the ones that work for us and left out the things that wouldn’t. We iterated quickly, recieved feedback with an open mind, and then acted on that feedback. If that’s not Agile, then I don’t know what is. 

Of course, I still need to find a way to teach people how to test drive their code, but that’s a problem, and topic, for another day. 

Semper Cogitet. 

, ,

3 Comments

Replacing simple factories with Func<TResult> delegate

I’ve recently noticed the emergence of a recurring pattern in my code. Each project has the same class in it, and this duplication has really been bothering me. The solution to this repetition just hit me this morning while I was reading Jon Skeet’s “C# In Depth.”

First, let me illustrate the pattern I was using and why it’s a problem.

I’ve been refactoring 1-tiered legacy code into something testable. This has involved creating a data layer class, typically called DataProvider which implements an IDataProvider interface so that I can easily mock out the data access.  Now, because any kind of data access is extremely likely to have unmanaged resources, IDataProvider extends IDisposable.

Read the rest of this entry »

, , , , , , ,

Leave a comment

Comments on Doc Comments

I was reviewing some VBA code over at Code Review Stack Exchange the other day and it got me to thinking about comments. The code wasn’t too bad, but there were so many comments in the code that, well…

I just read over this again and I think the fact that I mention the comments so often really highlights the issue. I barely even looked at the code because I was too distracted by all the comments.

Read the rest of this entry »

, , , , , ,

Leave a comment

That’s not real experience

I recently had a pretty big change in my life. My company went through an internal reorganization. I was left with a choice of moving my family a few hundred miles or finding new work. Since the prior wasn’t really an option for us, I took the opportunity to make a career move. 

Now was as good a time as any. Luckily, I didn’t have too much trouble finding a replacement job, but finding it was quite an experience. It turns out that nobody is interested in hiring a VB6 developer these days, and if they are, they want to make your job title “Excel Analyst” and severely underpay you. Seriously people, a good VBA dev is worth their weight in gold compared to all the people writing spaghetti with the macro recorder. Find a real programmer and pay them what they’re worth. You may spend more now, but it will save you tremendously in maintenance costs. But I digress…

VB6 was replaced by .Net twelve years ago, so I was looking to move on from VBA development and (professionally) into .Net development anyway. Getting my foot in the door without any professional .Net experience turned out to be quite a task though. I can’t tell you how many times I heard a recruiter or HR screener say, “But where is your .Net experience. All I see is this open source project. Don’t you have any real experience?” Even now, months later, that statement makes my blood boil. Let me tell you folks, experience writing open source software is real experience. The last time I checked, Rubberduck was at over 35,000 lines of code excluding the ANTLR generated code. It’s as large and complex as any CRUD app I’ve created in 5 years of professional software development, if not more so. Open source development teaches you more than just code though. It teaches you a number of other things as well. 
Read the rest of this entry »

3 Comments

VBA and Git the Sequel: This Time it’s Integrated

I’ve talked before about using Git with VBA and also about the importance of using source control and other tools for our work. Truthfully though, using source control with VBA is still hard. This is mostly because getting the code modules into and out of the VB Project is hard, and harder to do right. Well, you may have heard that there’s a new duck in town. It’s taken me about 6 months of spare time, but Rubberduck v1.4 not only has a source control COM library that you can use right in VBA to work with Git VBA repositories, but you can also now branch, commit, pull, push, and merge right from inside of the editor.

Read the rest of this entry »

, , , , , ,

2 Comments

Follow

Get every new post delivered to your Inbox.

Join 441 other followers