Re: [OctDev] Analyze75

[Carnë D. <car...@gm...>]

On 26 September 2012 11:10, adam aitkenhead <ada...@ho...> wrote:
>> From: car...@gm...
>> Date: Tue, 25 Sep 2012 19:34:15 +0200
>> Subject: Re: [OctDev] Analyze75
>> To: ada...@ho...
>> CC: oct...@li...
>>
>> hey, please avoid top posting. Reply at the bottom
>>
>> On 25 September 2012 17:07, adam aitkenhead <ada...@ho...>
>> wrote:
>> >> From: car...@gm...
>> >> Date: Tue, 25 Sep 2012 16:30:48 +0200
>> >
>> >> Another thing I noticed is the many ways it tries to handle invalid
>> >> filenames. While it looks kinda slick, I'm not sure it's a good idea
>> >> to have the function doing it, it should be done by the script that
>> >> calls it. When you give wrong arguments to a function, trying to guess
>> >> what is right can lead to weird results. Or does matlab also behaves
>> >> that way (it's not documented at least). My suggestion would be to
>> >> maybe display the file selection if no argument is given but return an
>> >> error if the filenames are not valid files. What do you think?
>> >>
>> >
>> > All sounds good to me. Makes sense to keep to the standard way of doing
>> > things to make it behave as others might expect. I'm not all that
>> > familiar
>> > with the standard (yet), so if you see any ways to standardise it that
>> > is
>> > great.
>>
>> I've made a few more changes. Could you please give them a try to make
>> sure I didn't broke anything? :p
>>
>> I've moved the code that was common between read and info into a
>> shared private function, checks after fseek and fopen, everything in
>> one function (I see no need to have subfunctions since they are only
>> called once anyway), replaced a long if block for a switch block,
>> changed many checks for simpler, used dir to check the file size
>> (rather than the low level fseek and ftell), and a lot of whitespace
>> to make it pretty and extended documentation. I tried to make the code
>> run in matlab (though I'd guess a matlab user would actually just use
>> the matlab function).
>>
>> Here's the files:
>>
>> https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/private/analyze75filename.m
>>
>> https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/analyze75info.m
>>
>> https://sourceforge.net/p/octave/code/11114/tree//trunk/octave-forge/main/image/inst/analyze75read.m
>>
>> > About the input checks - I'd aimed to make it behave as Matlab does for
>> > all
>> > acceptable inputs. But for the cases where Matlab throws an error, this
>> > function generally asks the user what to do instead. But I'm happy for
>> > this
>> > to be simplified or left for other functions which call these.
>>
>> Done.
>>
>> Carnë
>
> Changes look good.  Few minor edits to analyze75filename.m (see below), but
> otherwise everything works well.  Thanks for the help tidying it up!
>
> -  function filename = analyze75check (filename)
> +  function filename = analyze75filename (filename)
>
> -  elseif (~exist (filename,'file')
> +  elseif (~exist (filename,'file'))
>
> -  error ('analyze75info: no file `%s''', filename)
> +  error ('analyze75: no file `%s''', filename)

Thanks. Fixed