Re: [Pyobjc-dev] pyobjc on 2.2.1 & non-framework builds
Brought to you by:
ronaldoussoren
From: Bill B. <bb...@co...> - 2002-10-25 01:34:17
|
On Thursday, October 24, 2002, at 04:47 PM, Jack Jansen wrote: > On maandag, september 23, 2002, at 07:13 , Bill Bumgarner wrote: > >> I'll have to revisit Jack's PythonLauncher when I have more time-- it >> has some serious issues/bugs. > > Bill, > could you explain, please? PythonLauncher is my first "real" ObjC > program, so I'm not surprised there's bug in it (let alone > ideosyncracies: I couldn't find any example of how to mould the > per-file settings based on preference settings in the Document > paradigm, so I just came up with something). If you can give me some > hints as to what I did wrong I'd love to hear it... Jack, It works -- which is more than can be said for a lot of first programs written by developers working against an unfamiliar API! A lot of the problems have to do with not following the patterns as set forth in "traditional" ObjC/Cocoa programming -- not something that someone new to the environment would necessarily find natural to their development environment. Your skin is thick, you know you're a damned good at this stuff, and I don't have a lot of time -- as such, I'm not going to candy coat anything... just try to stick with terse [hopefully] constructive criticism in the form of a basic code /architecture review. Low level code review (syntax, naming, etc -- i.e. nit picking): The methods ... - (IBAction)do_run:(id)sender; - (IBAction)do_cancel:(id)sender; - (IBAction)do_reset:(id)sender; - (IBAction)do_apply:(id)sender; ... would be traditionally declared as (keeping the names the same, but in the ObjC style) ... - (IBAction)doRun:(id)sender; - (IBAction)doCancel:(id)sender; - (IBAction)doReset:(id)sender; - (IBAction)doApply:(id)sender; ... the 'do' prefix is a useful distinction. It is equally as common to see no prefix/suffix or to see 'Action:' as a suffix on each action method (redundant in the declarations, damned handy -- like do* -- in the code): - (IBAction)runAction: sender; - (IBAction)cancelAction: sender; - (IBAction)resetAction: sender; - (IBAction)applyAction: sender; - In the init method ... - (id)init { [super init]; if (self) { script = @"<no script>.py"; filetype = @"Python Script"; } return self; } ... the two instance variables need to be retained and the test for (self) being defined will never fail even if super's init did fail. Try... - (id)init { self = [super init]; if (self) { script = [@"<no script>.py" retain]; filetype = [@"Python Script" retain]; } return self; } ... or, alternatively, simply set script and filetype to nil (or, if feeling particularly terse, don't set 'em at all because ObjC initializes all variables to nil upon allocation -- personally, I like to set it to nil simply as an acknowledgment that that is what I expect the value to be). - The methods ... - (void)load_defaults - (void)update_display - (void)update_settings ... would typically be declared as ... - (void)loadDefaults - (void)updateDisplay - (void)updateSettings ... etc ... - In general, you should use -UTF8String to retrieve character buffers from NSString objects to be passed off to the BSD layer. The BSD layer should be able to handle UTF-8 encoded unicode for paths, etc, throughout -- by doing this, the app will work on systems where the app, python or the scripts are installed on a path that has unicode characters in it. - In -readFromFile:.... script = [fileName retain]; filetype = [type retain]; Assuming the previous values of the iVar have been retained (the bug in init has been fixed), this will leak the two string objects. Release 'em first. - FileSettings Lots of hardcoded stuff throughout -- need to be able to support a framework build of python in ~/Library/Frameworks/ or a command line in /usr/bin/python, /usr/local/bin/ or /sw/bin (or wherever). - The interaction between -init, -initWithFileSettings: and -factorySettingsForFileType: is a bit odd. If anything, factorySettingsForFileType: should return void and -init should simply invoke the method before returning self. -factorySettingsForFileType: should probably be -applyFactorySettingsForFileType: since it is more about changing the instance and less about finding a settings set for a particular file type (which would be the role of a class method). - -initWithFileSettings: That style of indirection into the object (i.e. source->debug) is extremely uncommon in ObjC. It breaks encapsulation. Any method prefixed with -init is generally considered to be an initializer and, as such, is likely going to be used in the pattern of [[... alloc] init...] -- as such, the method should invoke [self init] or [super init]... --- In interacting with user defaults, I typically create a property list file that contains the registration defaults for the application. This file is added to the project's resources. To load the defaults is a matter of... ... typically done in applicationDidFinishLaunching: or applicationWillFinishLaunching: ... NSDictionary *registrationDefaults = [NSDictionary dictionaryWithContentsOfFile: [[NSBundle mainBundle] pathForResource: @"RegistrationDefaults" ofType: @"plist"]]; [[NSUserDefaults standardUserDefaults] registerDefaults: registrationDefaults]; This also makes it quite easy to restore all the way back to factory settings without hardwiring the factory settings into the code -- simply grab the registration defaults from the file and set them into the app's domain. --- Bugs & Architecture: - there should be a way in the GUI to open a file into the settings -- i.e. the Open... command should be "Run script..." and "Open..." should truly open the script as a document. Or maybe have an accessory view with an "edit settings before running" checkbox? - the 'run in terminal' button doesn't update the field contents and, as such, the contents can get out of sync... - NSTask would be a better candidate than system() to subshell out the command. It would allow the app to monitor the output and display a console. If motivated, you could even have a field via which to interact with the interpreter (the current interactive mode doesn't do much good if, say, the app were launched from the Finder). - If the multi-doc architecture is going to be used, there should be something to save! How about saving the current settings out to a file such that the user can double-click the file to not only run the script but run it with their custom set of settings? - If I use the new menu item, it doesn't seem possible to set the script to be executed by the document. - +getDefaultsForFileType: should use a dictionary to store the file settings. - [[FileSettings new] init] == [[[FileSettings alloc] init] init] - Since both the preferences window and the documents have the same interior UI, the NIB file for that UI should be split out and the preferences and document window should share that single NIB. The preferences window should likely move to a tabbed view with the different file types in the tabs -- that seems to be the HIG way these days. Finally, the app really isn't a multiple-document type app. The comment above would move it in that direction, but there still wouldn't be a concept of "documents" given the current implementation. As it is, the app's name is indicative of it's role -- it launches Python scripts and provides a bit of control over how they are executed. The first step in moving to a true multi-doc style app is for the app to give the user control over when documents are closed -- that is, if a user opens a python script [as opposed to Running... a script without opening a visible document at all], the associated window should remain open until the user tells it to go away. The document should have a notion of 'currently running, do you want to launch another instance of the same thing?' and 'abort/stop current run'. Documents should be able to be saved -- everything is pretty much there to do so now, just a matter of returning an NSData containing the settings + the path to the script from the appropriate NSDocument method. ... My train has arrived at its destination, so I'm going to send this now... Given the direction that MacPython is going-- away from solely focused on the framework build and to a form that works with Apple's build of python or whatever happens to be present-- there are some other things that should likely change about the GUI tools used to work with the Python environment. Of course, a part of me wants to rewrite PythonLauncher using PyObjC :-). b.bum |