Stef Done - https://pharo.fogbugz.com/f/cases/20363/ThemeIcons-downloadTo-has-an-argument-which-is-never-referenced-in-code
What really puzzled me is that this situation was not picked up by a code critic. 'Local variable declared and not referenced' is a common critic message, and this looks like a parallel situation. But I don't think this can be called a bug in the code critic system - can it? Peter Kenny -----Original Message----- From: Pharo-users [mailto:pharo-users-boun...@lists.pharo.org] On Behalf Of Stephane Ducasse Sent: 31 August 2017 19:16 To: Any question about pharo is welcome <pharo-users@lists.pharo.org> Subject: Re: [Pharo-users] Code mystery Can you open a bug entry? I can tell you that we clean a lot but human activity will always leads to glitches. On Tue, Aug 29, 2017 at 1:23 AM, Dimitris Chloupis <kilon.al...@gmail.com> wrote: > You are not going crazy that's an ugly method. Probably the author > intended to use dir and the forgot about it and instead hard coded the > dir path inside a class method. A mistake that can happen to anyone. > > The method will have to be updated anyway because name is to be > removed, so it won't work. > > As always too much code too few people. If you think that's bad > embrace yourself if you try to read Morphic code. Huge suffering for > me when I tried to learn how the task bar works and apparently using > it wrong it freezes the image. > > Tons of Pharo code needs a clean but needs also a lot more people. > On Tue, 29 Aug 2017 at 01:52, PBKResearch <pe...@pbkresearch.co.uk> wrote: >> >> Hello All >> >> >> >> Following the discussion on dark mode, I was browsing the code on >> themes (in Moose 6.1 = Pharo 6.0, Latest update: #60486). In Class >> ThemeIcons, I found this method: >> >> >> >> downloadTo: dir >> >> | zipArchive | >> >> >> >> zipArchive := self class destinationPath / (self name, >> '.zip'). >> >> zipArchive exists >> >> ifFalse: [ >> >> ZnClient new >> >> url: self >> url; >> >> downloadTo: >> zipArchive ]. >> >> >> >> ^ zipArchive >> >> >> >> The mystery is that the argument dir is not referred to anywhere in >> the code. It probably works, because the only invocation of the >> method is from >> ThemeIcons>>downloadFromUrl, which sets the argument from self class >> destinationPath, and the code above recreates this as the path to >> zipArchive. >> >> >> >> I thought I understood Smalltalk coding fairly well, but this really >> puzzles me. Why would anyone code like this? Shouldn’t it be picked >> up by a code critic? Or am I going crazy? >> >> >> >> Any help gratefully received >> >> >> >> Peter Kenny
