[Sergio Tapia]

New kid on the block at my job, barely on my second week.

I'm working a project that's not too complex, but it has it's serious issues. For example, controls on the form that aren't visible when running that instead of being removed were just .Visible = false;, multiple classes with 9000+ lines of code.

No use of classes as far as I can see, only a few to handle the data connections.

Just a cesspit that was left by an older developer who left with a grudge against the company.

So seriously, how do I get a handle on things? I've tried following the boyscouts code of always leaving the campsite cleaner than when you found it, but more times than not I find any changes breaking the entire build.

The there's the problem of many controls that seemingly were made for the same purpose being named txtUserName, txtUserName1, txtUserName2, and I can't find which one is being used.

How do you tackle these problems?

[macosxnerd101]

There is a saying in the industry about writing code. It states to always write code as if the person maintaining it is a psychopath that knows your address. Now, do you know this guy's address?

On a more serious note, this is why crappy code is kept in existence so much. Poorly written code propagates poorly written code. The other option is to revamp the entire program, which costs a lot of time, energy, and money. If the code is truly that bad, try and feel out your coworkers (the programmers) about an overhaul of the program. If they aren't opposed to it, perhaps proceed to management. Since you've only been at your job for a couple weeks, make sure you understand the ramifications on the company before approaching the overhaul. This will help you make a more informed decision, and better be able to communicate the need for this to management.

Hope this helps some.

[Nightfish]

Well, that's a toughy. What you can do probably depends a lot on what your job description says and how much leeway people are going to give you. Also how long you think you'll have to work with this.

If this is a really long term thing you might want to gently push towards rewriting / restructuring this. Depending on who you have to pitch this to, you might have to prepare a pretty good case of why this is necessary even though the program "works". Sort of.

If you're only there for a short peroid of time, just patch things up as best as you can and move on. Pick your battles, is what I always do. You mentioned you were probably moving to the states soon-ish in another thread, no? So maybe you don't want to get too invested in this.

A few things you can probably solve rather easily is stuff like "txtUserName, txtUserName1, txtUserName2". Just try to figure out what one of them does and rename the thing to something proper. Repeat for the rest of the variables.

At the end of the day, I guess software projects are very rarely handled the way they should be. Around here it's because you have a really hard time justifying to the management a few weeks worth of work that produces no code at all (analysing and planning, etc).

This post has been edited by Nightfish: 29 July 2011 - 08:18 AM

[Sergio Tapia]

I just don't want to be one of those douches that comes in and thinks he's a hot tamale and demands changes. I've seen those guys and I always felt annoyed by them. In this case I think some changes are in order but I just don't want to come across as a douche to everybody (including myself).

[macosxnerd101]

That's why you talk with your coworkers first and feel them out. Do they react negatively to the code? Also, just be matter of fact and focused on the company's success. Talk about benefits and improvements, not about how much something sucks. That shows interest in the company rather than arrogance or douchebaggery.

[DreamyOR]

Well, besides the advice that someone could give from his experience i will suggest a book that tries to answer this question in a very effective way.
Code Reading: The Open Source Perspective

This post has been edited by DreamyOR: 29 July 2011 - 09:06 AM

[Kilorn]

I agree with mac on this one. If you do propose the idea of re-working the code, the best approach is to state the benefits of doing so. You don't want to go in there and talk about why it's wrong but rather how to make it better. Keep the focus on the positive side and you won't come across as douchey, but rather someone who cares about the product they are working on.

Also, I love that new signature, Sergio.

[Martyr2]

I feel for you, I have been in the situation many times and I follow this strategy which has worked out rather well:

1) Feel out the co-workers to see how flexible they are about reworking code (as already mentioned). If they are fairly open, good. If not, do what you can and talk to a supervisor about it.

2) Once you have buy in, look for the most bang for your buck code segments and start with them first. The idea here is that you want to rework something that will reinforce others that the code can really benefit from change. If you see a class that can be optimized for a 125% speed improvement by changing one line, awesome!

3) Work from top priority classes down to less priority, cutting and slashing first and then optimizing. I call this the "cut the fat" section of a code rework. If there are controls that should be removed and never shown, axe them. Can you refactor 400 lines into 4 lines? Awesome, do it. Once you axe enough code and get to the heart of what it does, then optimize (not too much) and move on. Just to give you an idea, I took on an intranet once that had over 800 files. After my "cutting the fat" I was down to 600. Then I went through those 600 and optimized things and improved the site's speed by about 25%.

4) So now you are lean and mean with things performing well. This is the point where you go to the supervisor and show the improvements. After they are done jumping around, you then are in a great position to propose design changes for even more savings. By this time they will be more than open to your ideas.

Hope this helps!

P.S. Be sure to back things up as you go along cutting the fat, just in case you cut something that should have indeed been there.

This post has been edited by Martyr2: 29 July 2011 - 09:39 AM

[Kilorn]

Sounds like a good time for a raise and/or promotion as well.

[DivideByZero]

Luckily I haven't experienced this at my current job yet, although there is a design choice I'm not ecstatic about, but I can deal with it.

If I did, I'd probably ask the person that wrote the code to explain to me why s\he did certain things instead of doing what I thought would be better suited for the situation.
That way you sound curious instead of arrogant

This post has been edited by DivideByZero: 29 July 2011 - 09:51 AM

[xclite]

I sit in the corner and cry. Government-produced software is terrifying.

[Nightfish]

Sergio Tapia, on 29 July 2011 - 08:38 AM, said:

I just don't want to be one of those douches that comes in and thinks he's a hot tamale and demands changes. I've seen those guys and I always felt annoyed by them. In this case I think some changes are in order but I just don't want to come across as a douche to everybody (including myself).

Well, the thing is, everyone thinks like that. I doubt anyone who proposes changes doesn't really think change needs to be made.

Anyway, with the information you have provided, I don't think it's possible for anyone to give you perfect advice. The surrounding factors aren't really clear so people might be tempted to fill in the blank spots with what they experienced. I do that, too. I'm in a somewhat similar situation, got added to an existing team that has an MO that can only end in disaster.

What I did was to carefully probe why things were done like this and what people thought. Like you said, when you're new, you don't want to cause too many ripples in the pond. Especially if the pond is a cesspool and filled to be level with your lower lip. Ho boy... No splashing please. Anyway, more likely than not there is a "good" reason for things being the way they are. But it's up to you to feel that out.

If you do push for a rewriting of code and have to pitch that to management or any non-programmer types, do stress how much easier this will make future maintenance. Also, don't neglect to say that this is along the lines of an investment. Yes, it takes longer to do things right than to just duct tape them together, but it's a long term solution. I find the duct tape analogy gets me far in things like this. People can related to duct taping something that's broken on their car and they understand that it's okay as an interim thing but it's neither pretty nor lasting. Nor can it be incorporated into a true solution.

[Nakor]

Something to keep in mind about the Visible=false, if you're using ASP.Net, is that it is an effective way to lazy-load databound controls. When a control's visibility is set to false in this manner it also prevents the control from being databound which can radically improve page load times depending on how many controls and how much data is being loaded onto the page. You instead load the data on command by setting the visible property back to true and calling the control's databind method when the user clicks a button or selects the tab the data is within, etc.

However if they just set the visibility to false and then do nothing with it then that control should probably just be removed. It's kind of like some code I've had to work with where a guy would change some code and instead of rewriting the code he would just comment out the old code and leave it there. There were pages with literally hundreds of lines of commented out code.

[Sergio Tapia]

Yes, that's the case here. There are classes with commented out code that counts in the thousands of lines. Almost 5000+ lines of code commented out.

[Programmist]

Having worked as a consultant for a few years now I can totally relate because I have been "that guy." People working at X company rarely want anyone to rock the boat, no matter how much their codebase sucks. They are often comfy in their tiny kingdoms and would rather maintain the status quo than bother learning anything new from an "outsider" - especially if some unpleasant work is required. Luckily, in many of these scenarios I've been specifically hired by senior management/owners to fix things, so I had the support I needed to affect change. If you don't have that support then your road ahead could be very difficult - especially if you have many "lifers" who just want to skate by.

If you do have support from management and buy-in from some peers then there are some principles that you can follow to make your life easier.

1. Increase test coverage before doing a major refactor. Depending on how crappy the code is, adding mocks and unit tests could be painful, but can increase your confidence in maintaining code integrity during any refactoring that follows.

2. Practice what you called the "boyscout" principle, but take it a step further. I've heard it called many things ("ratcheting" is one of them) but the basic idea is that you leave the code cleaner than you found it. So, if you are working on a story or a bug, you do your work and then fix/clean something else. I've even seen shops that use static analysis tools (e.g. Sonar) to enforce this policy by disallowing a check-in if the number of bugs (or code smells) on the developer's copy are >= those in the repo. That might be a bit harsh, but it's not unheard of.

3. Add peer code review as part of your process. Most of the places I've been use JIRA or some other ticketing system. So why not add peer review as part of the process somewhere between "development complete" and "QA." These reviews can be assigned randomly, so that no one feels they are being picked on. It might also be a good idea to establish some ground rules with respect to constructive criticism vs just being rude. Telling someone their code "sucks" is not going to help anyone.

4. Education is key to buy-in. I've seen this take the form of a "book club" where everyone is reading a book (sometimes 2-3 books) at their own pace and then contributes something they learned to the discussion during sessions that meet 1-2 times per week (usually while eating lunch). Contributions can be from any part of the book, so there's no mismatch between people of different reading levels and schedules. If you make it participatory and not instructor-led, it has a more casual feel and people are more likely to engage because they feel they are participating in a discussion and not being "preached at."

5. Hold team retrospective feedback meetings once per sprint/iteration. I'm not a big fan of meetings, but I've seen this work pretty well to quell discontent. These meetings should rarely include management, so that the team can feel free to voice any frustrations. It's also a place for them to applaud things that they think are great. I'd suggest handing out post-it notes and having each team member write one suggestion, complaint, good thing, or general comment about the previous sprint/iteration. These can be about anything that affects day-to-day operations (e.g. "the room is too hot", "I hate pair programming", "we need more snacks", "I don't understand what our objective is", "My manager doesn't listen to me." etc). These can be collected by the meeting leader, grouped in categories, and discussed. At the end of the meeting make sure to define a set of "actions." that should happen.

Anyway, those are just some ideas. Hope it helps.