1

I'm currently debugging a very large public method that does these things.

  1. Fetch record(s) from DB
  2. Do some logical checks
  3. based on logical checks, determine if additional DB operations are done like save/ update/ delete
  4. Call a service and start a scheduled service
  5. etc, etc

Now, within these logical groups, it fetches data, etc, etc. Our architect has advised to write unit tests against it. If i break them down into smaller public methods that are being called from the big parent method, i know that it wont be good practice. Ive done some research and people said about saying something about using internal then exposing the assembly using InternalsVisibleTo. The method has also been existing for quite some time now and i want to avoid totally refactoring it and do really mind blowing regression testing.

Can anyone give me advice?

update

public JobModel SaveAndUpdateJob(JobModel jobModel)
{
    using (var dbSession = OpenSession())
    {
        var jobEntity = isNew ? new JobEntity() : dbSession.Get<JobEntity>(jobModel.Id);

        jobEntity.JobUniqueCode = jobEntity.GenerateUniqueCode(jobEntity, IsPublished);

        RateJobAgainstSurvey(jobModel, dbSession, jobEntity);

        _emailService.SendNotification(dbSession);
    }
    return jobModel;
}
Nkosi
  • 235,767
  • 35
  • 427
  • 472
user1465073
  • 315
  • 8
  • 22
  • Your question is too broad. You have a function that does everything and doesn't already have unit tests, it seems unlikely that even if you break it into public functions you will be able to isolate functionality such as the DB to allow unit testing. A better approach would be to write integration tests for the section of code that hit an actual database, so that you have protection to allow you to do some actual beneficial refactoring of the codebase. It seems odd that your Architect's input is write some unit tests, rather than you need to clean the structure up in a safe way. – forsvarir Mar 18 '16 at 13:27
  • problem with this is that we dont have a properly setup integration testing mechanism. I've read abt SQLLITE but if i go that route that will be another round of coding. – user1465073 Mar 18 '16 at 13:54
  • What's your goal with adding the unit tests? The method has been around for a while, is it something that hasn't been tested at all, is expensive to test, is it often changing or is the code static, when it is updated does it often result in breakages and bugs, is it simply 'busy' work because you're between cycles and it's a task that the architect feels needs done, but can always be dropped if something else comes up? – forsvarir Mar 18 '16 at 14:20
  • after quite a number of refactoring, the method became brittle and because of the complex branching logic, QA testers cant seem to test the number of possible scenarios based on how the method was made and the countless stuff it does inside. – user1465073 Mar 18 '16 at 14:56
  • 1
    So, the QA problem doesn't go away, no matter what you do. If the behaviour is required and they can't test it then you have a possibly bigger issue. Without seeing the code, it's hard to advise on the *right* approach, but raising the visibility of methods so that they can be called out of context is as likely to cause issues as mass refactoring (although the timeline for issues showing up can be very different). A first step might be too create a helper class and pull the doing logic into public methods on that class. Then use that class via an interface from you orchestrating logic. – forsvarir Mar 18 '16 at 15:28
  • This would allow you to test you complex branching logic. The next stage would be to split the helper into sensible logical entities, updating the branching tests as required for any new types... However without the code, it's all speculation – forsvarir Mar 18 '16 at 15:33
  • 1
    ok then, ill try to paste the source code here in 60 minutes, just going to get my stuff. – user1465073 Mar 18 '16 at 16:04
  • Edit it into your question, don't add it as comment... – forsvarir Mar 18 '16 at 17:14
  • ok, ive updated the question, the code snippet above is like only 10% of the actual code which lots of branching IF-then-else, delete this from DB, send that email if this logical check is true, etc, etc. NHIBERNATE directly mixed into the method, service calls, other methods called that updates directly the DB, updates the model, etc, etc. – user1465073 Mar 18 '16 at 17:18
  • to add, some methods are private static, some methods are instance methods. It also directly calls session variables. and some of the methods are transaction DB updates. – user1465073 Mar 18 '16 at 17:21
  • Refactor your code. Put for example the logical checks in a separate class and test this class separately. You can also mark some methods as internal. Look at [this](http://stackoverflow.com/questions/15440935/how-to-test-internal-class-library) article. – Jeroen Heier Mar 21 '16 at 05:14

0 Answers0