2

I am noticing a weird mentality, when it comes to refactoring code, and I am not too sure about how I am perceiving it.

Overview of the current project:

A web application used as an extension to an existing 3rd party web application. The 3rd party application maintains schedules between customers and vendors. The extension allows for modifying days/times that the customer requests to re-schedule, change the current set of schedules in case of emergency, change a vendor if they can no longer be contracted, and create the appropriate billing files. As far as development, the team is setup to follow most of the ideas of Scrum.

The system was built under many assumed one-off criteria, mainly due the customers of the project not fully understanding their internal processes (due to internal training/turnovers). We have seen many areas, where there are checks based on containing some hard-coded string.

This brings us to the issue. There is a check for a specific vendor (we can call it SomeVendor), and the check is set for if name contains Some. Later as we gain and lose vendors, another vendor gets added as SomeOtherVendor. The problem with that is of course the check based on containing Some. SomeVendor is a specific case that needs to be checked, and their are other vendors that need to similarly be checked and branches in their own set of functionality. In finding those checks, we see that it was developed in respect to a schedule can be related to a vendor but is not always assigned to them. The users would associate it during run-time.

The customer realizes that there is an issue with SomeVendor and have requested a defect be put into the backlog and have prioritized it with the product owner. I picked up the defect and developed a lookup table for the valid SomeVendor vendors (in case more than one was ever valid based on Id).

There is a mid-Sprint deployment coming up, so I went to discuss if there would be a need to add this to the deployment with my lead. He expressed that he liked the solution but asked if I could make it in a way that would incorporate mapping all vendors that need to be checked. He also stated making it solely for the current issue.

The following day the senior developer asks to work with me on the design and proceeds to discuss configuring all of the vendors and incorporating that into the codebase. As stated above, the system accounts for some of the vendors as they "can be" of some Type, but it is not always the case.

We discussed the point that the system only accounts for it as a can be, but it is not determined until they save at run-time. The suggestion is to bring this to the product owner and get the criteria from the project customers.

I see the point in refactoring to a point where the codebase would use this correctly, but I feel like it is stepping into the realm of over-engineering at this point. That feeling is mainly due to a story/defect has already be determined and prioritized with the customer, and we have given what we said we would do from the Sprint planning. This feels like we are sneaking in a deliverable that they don't know that they need (which has been discussed recently between us and the customer).

Is this type of refactoring something that should be happening, or is this a sign of bad technical pushback? In reality, it does not matter to me if the answer specifically deals with agile. It seems odd to me in the sense of the team having some requested work then delivering something that wasn't agreed.

1 Answer 1

Reset to default
4

I don't understand specific problem you have, but I noticed some general ideas in your question.

If the changes you are implementing are not affecting the user-percieved behavior of the product, then there is no need to involve product owner. It is your responsibility, as the technical expert here, to decide if this change is valuable for you in the long run. If spending few days now will save you weeks of debugging and bug fixing in the future, go for it. There should be no need to explain this to your product owner.

Another thing is calling what you are doing a refactoring. A refactoring is short and quick change, that can be immediately committed into the product, without any percievable change to the end user. And many of those small refactorings might drastically change internal workings of the software over long periods of time. What you are doing sounds more, like I call it, a re-engineering. But still, this change should not be seen by end-user.

I'm sure your change CAN be split into many small changes, that can be slowly deployed over time. This will allow you to check if any of the changes break something. It also gives other developers chance to better absorb the changes and provide some feedback.

mid-Sprint deployment

What is this? There should be no deployments during sprint. Only potentially deployable artifacts are created at end of the sprint. Everything else should be treated as work-in-progress. If you are making deployments during sprint, then I wonder why are you even using sprints, and not something continuous, like Kanban.

8
  • As far as the specific issue, I see the technical side where we would be making a change that the customer shouldn't see, but the change that would be made required discussing with the customer about mapping a relationship in data and determining how that relationship should work beyond the mapping (EX: when a billing file is made should we say that since the vendor is tied to a type specifically). And that, I feel makes it where we cross the line of something the customer will/will not see, since the "fix" is not going beyond what they asked.
    – eparham7861
    Commented Sep 17, 2017 at 17:11
  • For the mid-Sprint item, that is just where they are not following the methodology exactly but end up using agile, sprint, and so on as Buzz words, and that is a whole other thing that needs help.
    – eparham7861
    Commented Sep 17, 2017 at 17:13
  • 3
    @eparham7861 It seems to me, that in process of improving internal quality of your code, you found out issue in the domain. This sometimes happen. And it is actually good thing, as it allows you to better express the domain concepts in the code. If that is the case, you should definitely point this out to the product owner. The PO didn't request it, because they simply never had to express their knowledge in strict language that is code. It might have been something that is "assumed" and never really thought about explicitly.
    – Euphoric
    Commented Sep 17, 2017 at 17:16
  • Ok, that part felt right, and I am glad that we came to the decision of bring in the product owner into the discussion. The part that got me thinking was the attempt to push this kind of review and change without that discussion and under a work item that was stated for something specific by the product owner and customers. Thank you, and I believe that you answered my question.
    – eparham7861
    Commented Sep 17, 2017 at 17:22
  • 1
    Ugh. You totally had me until you said there should be no midsprint deployment. No where, and I do mean no where, in the Scrum guide does it say you can only deploy at the end of a sprint. Heck, it doesn't say you have to deploy at the end of the sprint either. Only that you must have a potentially releasable increment. Deploy as often as makes sense. Don't make your customers wait 2 weeks to get a fix for a bug that was done on day one of your iteration. end rant
    – RubberDuck
    Commented Sep 17, 2017 at 23:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.