{
  "$type": "site.standard.document",
  "bskyPostRef": {
    "cid": "bafyreihfzmfxwsguaebc65svtnyr7u2fp4333udttxpmapwczmezfaywpa",
    "uri": "at://did:plc:vyjlfm46mfv6u4vjp6qtrfx2/app.bsky.feed.post/3moxzokh2rst2"
  },
  "coverImage": {
    "$type": "blob",
    "ref": {
      "$link": "bafkreidg2k33tiuupnstmiowh7hyyvtsbe3gyhfygiytrzynysnhoctrxi"
    },
    "mimeType": "image/jpeg",
    "size": 81260
  },
  "path": "/articles/do-a-lot-to-do-nothing",
  "publishedAt": "2026-06-23T06:30:00.000Z",
  "site": "https://thedailywtf.com",
  "tags": [
    "CodeSOD",
    "Learn more."
  ],
  "textContent": "Today's anonymous submitter works in finance. I'll let them start the introduction:\n\n> This is a legacy application that has been running for nearly a decade in production so one could say that it's been thoroughly tested by daily production use and nothing needs changing\n\nThis is a collection of two C# methods, and we'll start with `ValueAGPFund`, which isn't a WTF per se, but definitely not code I'd want to maintain either.\n\n\n    public Valuation ValueAGPFund(int valuationId, ValueAFundParameters parameters, CapitalAccount capitalAccount, int? lotId)\n    {\n        if (parameters.UseActiveCoefficientSet)\n        {\n            parameters.CoefficientSet = _coefficientSetQueries.GetActive();\n        }\n        parameters.InternationalDveCoefficientSets = _coefficientSetQueries.GetInternationalDveActive();\n        var referenceData = _referenceDataFactory.CreateReferenceData(parameters, capitalAccount);\n        if (lotId != null)\n        {\n            var di = referenceData.FundDirectInvestments.Where(x => x.PositionId == lotId);\n            referenceData.FundDirectInvestments = di;\n        }\n\n        var countryMappings = _countryQueries.GetFullIsoCountryList();\n        var valuation = _valuationFactory.Initialise(referenceData, parameters, countryMappings);\n        valuation = ApplyValuators(valuation, referenceData, _valuatorFactory.CreateValuators(valuation, this));\n\n        var valuationForCoverage = _valuationQueries.GetWithDirectValuationsAndFundValuations(valuationId);\n        valuation = ApplyCoverage(valuation, valuationForCoverage);\n\n        foreach (var fv in valuation.FundValuations)\n        {\n            _logger.Info($\"Debugging distributions: for fund (parameter fund id = {parameters.FundId}, valuation fund id = {valuation.FundId}, fund valuation fund id = {fv.GpFundId}) in valuation {valuationId},\" +\n                $\" loaded fund investment distributions from {string.Join(\", \", fv.FundInvestmentDistributions.Select(x => $\"{x.InvestmentId}:{x.TransactionDate:yyyy/MM/dd}\"))}\");\n        }\n\n        foreach (var fv in valuation.FundValuations.Where(x => parameters.InvestmentIds.Contains(x.EqtInvestmentId)))\n        {\n            fv.ValuationId = valuationId;\n            _fundValuationCommands.Add(fv);\n        }\n\n        foreach (var dv in valuation.DirectValuations.Where(x => x.LotIdDiOnly == lotId))\n        {\n            dv.ValuationId = valuationId;\n            _directValuationCommands.Add(dv);\n        }\n\n        foreach (var vw in valuation.ValuationWarnings)\n        {\n            vw.ValuationId = valuationId;\n            _valuationWarningCommands.Add(vw);\n        }\n\n        var previousValuation = CheckPreviousValuationIfRequired(valuationId, parameters, capitalAccount, lotId);\n\n        if (previousValuation != null)\n            valuation.ChildValuations.Add(previousValuation);\n\n        if (parameters.Frequency == ValuationFrequency.Daily)\n        {\n            var unapprovedValuations = _valuationQueries.GetList(valuation.FundId, valuation.ValuationDate, valuation.Frequency, valuation.Purpose)\n                                                        .Where(x => x.IsApproved == ValuationStatus.Unapproved)\n                                                        .ToList();\n\n            _valuationCommands.Delete(unapprovedValuations.Select(x => x.Id).ToArray());\n        }\n\n        valuation.Id = valuationId;\n        _valuationCommands.Update(valuation);\n        _valuationCacheService.Refresh(valuation.Frequency, true);\n\n        return valuation;\n    }\n\n\nThe key problem with this function is that it's got loads of side effects. It modifies the `parameters` parameter, which while it was passed by value, the value itself is a reference, so you _are_ updating it on the caller, whether the caller likes it or not. It also modifies a bunch of internal class members. It's also just… doing a lot of different steps. It's not a WTF, but it's bad code. Note the call in the middle to `CheckPreviousValuationIfRequired`- we're going to come back to that in a second.\n\nLet's take a look at how it's called.\n\n\n    private Valuation CheckPreviousValuationIfRequired(int valuationId, ValueAFundParameters parameters, CapitalAccount capitalAccount, int? lotId)\n    {\n        if ((parameters.Frequency == ValuationFrequency.Quarterly || parameters.Frequency == ValuationFrequency.Monthly)\n            && ValuationPurposeHelper.UserGenerated(parameters.Frequency).Contains(parameters.Purpose))\n        {\n            var inPeriodParams = new ValueAFundParameters\n            {\n                FundId = parameters.FundId,\n                ValuationDate = parameters.ValuationDate.GetPreviousValuationDate(parameters.Frequency),\n                CreatedBy = parameters.CreatedBy,\n                Purpose = ValuationPurpose.InPeriodCalculation,\n                Frequency = parameters.Frequency,\n                InvestmentIds = parameters.InvestmentIds,\n                UseActiveCoefficientSet = true,\n                UseAmericanDve = parameters.UseAmericanDve,\n                ValuationOptions = parameters.ValuationOptions\n            };\n\n            var openingValuation = _valuationQueries.GetInPeriodOpeningValuation(inPeriodParams.FundId, inPeriodParams.ValuationDate, valuationId);\n\n            //return openingValuation == null\n            //       ? null\n            //       : ValueAGPFund(openingValuation.Id, inPeriodParams, capitalAccount, lotId);\n            return openingValuation == null\n                    ? ValueAGPFund(openingValuation.Id, inPeriodParams, capitalAccount, lotId)\n                    : null;\n        }\n\n        return null;\n    }\n\n\nThis function checks the input parameters. Depending on the values, it will either return null, or it will call `ValueAGPFund`. Wait a second, `ValueAGPFund` calls this function. That's not good.\n\nBut let's really focus in on the return statement and its comment:\n\n\n            //return openingValuation == null\n            //       ? null\n            //       : ValueAGPFund(openingValuation.Id, inPeriodParams, capitalAccount, lotId);\n            return openingValuation == null\n                    ? ValueAGPFund(openingValuation.Id, inPeriodParams, capitalAccount, lotId)\n                    : null;\n\n\nThe _current_ version checks if `openingValuation` is null, and if it is, tries to access it, thus triggering a `NullReferenceException`. This function either returns `null` or throws a `NullReferenceException`. So all that worrying about side effects and circular calls doesn't matter, but this likely isn't correct. The comment indicates that there _used_ to be a correct version, which only called `ValueAGPFund` if the valuation wasn't null- but that version likely had all the problems of circular calls and unpredictable side effects.\n\nAs it stands, the _application as a whole_ works. Since `CheckPreviousValuationIfRequired` only ever returns null or throws an exception, and since `ValueAGPFund` is only called from here, it _looks like_ these functions could just both be removed without problems. But our submitter is wary of doing that:\n\n> The problem is that I first need to figure out whether 1) this piece of code produces any side effects and 2) nobody is relying on the System.NullReferenceException being thrown here.\n\nNo worries, though, right? I'm sure your unit tests will catch any regressions caused by removing that. Because this is the kind of code that definitely has great unit tests.\n\n[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.",
  "title": "CodeSOD: Do a Lot to Do Nothing"
}