Samenvatting van de 'onderhandelingen' over het 'principeakkoord' die gisteren afgerond werd: in principe goed dat ze elkaar eens ontmoet hebben. Je moet even graven voor een stavaza, maar Iraanse vice-MinBuz Kazem Gharibabadi zegt tegen Iraanse staatsmedia de 'technische gesprekken' met de VS erop zitten, en dat 'de onderhandelaars' hebben besloten dat er vier werkgroepen opgezet worden, te weten (even in het Engels): "Sanctions Termination, Nuclear Affairs, Reconstruction and Economic Development, and Monitoring and Implementation." Zoals bekend is het Iraanse kernwapenprogramma natuurlijk de hoofdkwestie, en dat ging gisteren meteen al lekker. De Amerikaanse delegatie beweerde dat Iran had ingestemd met het toelaten van inspecteurs van het Internationaal Atoomagentschap, maar kort daarna ontkende Iraanse staatsmedia die toezegging: "De eventuele aanwezigheid van inspecteurs moet worden vastgelegd in een definitieve overeenkomst. Een overeenkomst die, gezien de ervaringen met Amerika, onwaarschijnlijk lijkt." Ook beweerde de Iraanse hoofdonderhandelaar Ghalibaf gisteren tijdens zijn terugreis dat er alvast $12 miljard aan bevroren Iraanse tegoeden vrijgegeven zou worden.
In ander nieuws vreest Israël dat de deal de Iraanse positie in Libanon, en daarmee Hezbollah, significant zal versterken, terwijl het Israël de handen op de rug bindt. Axios schrijft: "Israëlische functionarissen vrezen dat de nieuwe afspraken de maandenlange inspanningen van de VS en Israël om Hezbollah te verzwakken en de invloed van Iran in Libanon te verminderen, zullen ondermijnen. Meer direct maken ze zich ook zorgen over tegenwerking vanuit Washington D.C. telkens wanneer ze een aanval op Libanees grondgebied willen uitvoeren, of over druk van Trump om zich terug te trekken uit Zuid-Libanon zolang de dreiging van Hezbollah nog bestaat."
Gisteravond kwamen Netanyahu en zijn MinDef Katz met een gezamenlijke verklaring waarin gesteld wordt dat de IDF aanwezig zal blijven, en zal blijven opereren in Zuid-Libanon: "De IDF zal vastberaden blijven optreden om bedreigingen tegen onze soldaten en burgers te neutraliseren, terroristische infrastructuur te ontmantelen en de veiligheidszone in Zuid-Libanon te handhaven." Afijn, we gaan maar weer eens live.
Social
UTRECHT (ANP) - De hoge temperaturen zorgen voor extra druk op veel werkvloeren en het is belangrijk dat werkgevers daar goed op inspelen en zich daarop voorbereiden. Dat melden de arbodiensten ArboNed en HumanCapitalCare. Zij stellen dat kleine aanpassingen al verschil kunnen maken, zoals het plannen van werkzaamheden op koelere momenten van de dag, het tijdelijk verlagen van de werkdruk of het inlassen van extra pauzes.
De diensten zeggen dat hitte directe gevolgen heeft voor mensen. Werknemers kunnen sneller vermoeid raken, zich moeilijker concentreren en minder goed herstellen gedurende de werkdag. "Dat vergroot de kans op fouten en onveilige situaties. Vooral in functies waarin veiligheid, alertheid en fysieke inspanning een belangrijke rol spelen, is dat een risico dat je niet moet onderschatten", zegt bedrijfsarts en directeur medische zaken bij ArboNed Redmer van Wijngaarden.
Hij stelt dat goede voorbereiding op hitte essentieel is. "Hoge temperaturen zijn iets waar je als werkgever structureel rekening mee moet houden. Ze komen elk jaar terug."
AMSTERDAM (ANP) - Heineken heeft Rafael Oliveira benoemd tot nieuwe topman van de bierbrouwer. Hij volgt topman Dolf van den Brink op, die in januari aankondigde te vertrekken. Oliveira was tot voor kort de topman van koffie- en theeconcern JDE Peet's, bekend van Douwe Egberts en Pickwick.
Van den Brink trad op 31 mei af, waarna het bedrijf zonder topbestuurder kwam te zitten. Hij blijft nog acht maanden in een adviserende rol beschikbaar voor het Nederlandse concern. Oliveira begint na een buitengewone aandeelhoudersvergadering op 1 oktober als hoogste baas bij de bierbrouwer.
Today's anonymous submitter works in finance. I'll let them start the introduction:
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
This 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.
public Valuation ValueAGPFund(int valuationId, ValueAFundParameters parameters, CapitalAccount capitalAccount, int? lotId)
{
if (parameters.UseActiveCoefficientSet)
{
parameters.CoefficientSet = _coefficientSetQueries.GetActive();
}
parameters.InternationalDveCoefficientSets = _coefficientSetQueries.GetInternationalDveActive();
var referenceData = _referenceDataFactory.CreateReferenceData(parameters, capitalAccount);
if (lotId != null)
{
var di = referenceData.FundDirectInvestments.Where(x => x.PositionId == lotId);
referenceData.FundDirectInvestments = di;
}
var countryMappings = _countryQueries.GetFullIsoCountryList();
var valuation = _valuationFactory.Initialise(referenceData, parameters, countryMappings);
valuation = ApplyValuators(valuation, referenceData, _valuatorFactory.CreateValuators(valuation, this));
var valuationForCoverage = _valuationQueries.GetWithDirectValuationsAndFundValuations(valuationId);
valuation = ApplyCoverage(valuation, valuationForCoverage);
foreach (var fv in valuation.FundValuations)
{
_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}," +
$" loaded fund investment distributions from {string.Join(", ", fv.FundInvestmentDistributions.Select(x => $"{x.InvestmentId}:{x.TransactionDate:yyyy/MM/dd}"))}");
}
foreach (var fv in valuation.FundValuations.Where(x => parameters.InvestmentIds.Contains(x.EqtInvestmentId)))
{
fv.ValuationId = valuationId;
_fundValuationCommands.Add(fv);
}
foreach (var dv in valuation.DirectValuations.Where(x => x.LotIdDiOnly == lotId))
{
dv.ValuationId = valuationId;
_directValuationCommands.Add(dv);
}
foreach (var vw in valuation.ValuationWarnings)
{
vw.ValuationId = valuationId;
_valuationWarningCommands.Add(vw);
}
var previousValuation = CheckPreviousValuationIfRequired(valuationId, parameters, capitalAccount, lotId);
if (previousValuation != null)
valuation.ChildValuations.Add(previousValuation);
if (parameters.Frequency == ValuationFrequency.Daily)
{
var unapprovedValuations = _valuationQueries.GetList(valuation.FundId, valuation.ValuationDate, valuation.Frequency, valuation.Purpose)
.Where(x => x.IsApproved == ValuationStatus.Unapproved)
.ToList();
_valuationCommands.Delete(unapprovedValuations.Select(x => x.Id).ToArray());
}
valuation.Id = valuationId;
_valuationCommands.Update(valuation);
_valuationCacheService.Refresh(valuation.Frequency, true);
return valuation;
}
The 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.
Let's take a look at how it's called.
private Valuation CheckPreviousValuationIfRequired(int valuationId, ValueAFundParameters parameters, CapitalAccount capitalAccount, int? lotId)
{
if ((parameters.Frequency == ValuationFrequency.Quarterly || parameters.Frequency == ValuationFrequency.Monthly)
&& ValuationPurposeHelper.UserGenerated(parameters.Frequency).Contains(parameters.Purpose))
{
var inPeriodParams = new ValueAFundParameters
{
FundId = parameters.FundId,
ValuationDate = parameters.ValuationDate.GetPreviousValuationDate(parameters.Frequency),
CreatedBy = parameters.CreatedBy,
Purpose = ValuationPurpose.InPeriodCalculation,
Frequency = parameters.Frequency,
InvestmentIds = parameters.InvestmentIds,
UseActiveCoefficientSet = true,
UseAmericanDve = parameters.UseAmericanDve,
ValuationOptions = parameters.ValuationOptions
};
var openingValuation = _valuationQueries.GetInPeriodOpeningValuation(inPeriodParams.FundId, inPeriodParams.ValuationDate, valuationId);
//return openingValuation == null
// ? null
// : ValueAGPFund(openingValuation.Id, inPeriodParams, capitalAccount, lotId);
return openingValuation == null
? ValueAGPFund(openingValuation.Id, inPeriodParams, capitalAccount, lotId)
: null;
}
return null;
}
This 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.
But let's really focus in on the return statement and its comment:
//return openingValuation == null
// ? null
// : ValueAGPFund(openingValuation.Id, inPeriodParams, capitalAccount, lotId);
return openingValuation == null
? ValueAGPFund(openingValuation.Id, inPeriodParams, capitalAccount, lotId)
: null;
The 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.
As 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:
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.
No 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.