Это хороший (одобренный Apple, одобренный Apple) класс моделей? - PullRequest
3 голосов
/ 28 февраля 2010

Я уже давно пользуюсь Objective-C, но не очень хорошо следую рекомендациям Apple.Недавно я прочитал Шаблоны проектирования какао и Руководство по реализации объекта модели , и я пытаюсь сделать некоторые очень простые вещи, но делать их очень хорошо.

ЕстьЯ пропустил какие-либо основные понятия?Пожалуйста, не упоминайте self = [super init];это уже много раз освещалось на SO.Не стесняйтесь критиковать мои #pragma mark s, хотя!

#import "IRTileset.h"
#import "IRTileTemplate.h"

@interface IRTileset () //No longer lists protocols because of Felixyz

@property (retain) NSMutableArray* tileTemplates; //Added because of TechZen

@end

#pragma mark -
@implementation IRTileset

#pragma mark -
#pragma mark Initialization

- (IRTileset*)init
{
    if (![super init])
    {
        return nil;
    }

    tileTemplates = [NSMutableArray new];

    return self;
}

- (void)dealloc
{
    [tileTemplates release];
    [uniqueID release]; //Added because of Felixyz (and because OOPS. Gosh.)
    [super dealloc]; //Moved from beginning to end because of Abizern
}

#pragma mark -
#pragma mark Copying/Archiving

- (IRTileset*)copyWithZone:(NSZone*)zone
{
    IRTileset* copy = [IRTileset new];
    [copy setTileTemplates:tileTemplates]; //No longer insertTileTemplates: because of Peter Hosey
    [copy setUniqueID:uniqueID];

    return copy; //No longer [copy autorelease] because of Jared P
}

- (void)encodeWithCoder:(NSCoder*)encoder
{
    [encoder encodeObject:uniqueID forKey:@"uniqueID"];
    [encoder encodeObject:tileTemplates forKey:@"tileTemplates"];
}

- (IRTileset*)initWithCoder:(NSCoder*)decoder
{
    [self init];

    [self setUniqueID:[decoder decodeObjectForKey:@"uniqueID"]];
    [self setTileTemplates:[decoder decodeObjectForKey:@"tileTemplates"]]; //No longer insertTileTemplates: because of Peter Hosey

    return self;
}

#pragma mark -
#pragma mark Public Accessors

@synthesize uniqueID;
@synthesize tileTemplates;

- (NSUInteger)countOfTileTemplates
{
    return [tileTemplates count];
}

- (void)insertTileTemplates:(NSArray*)someTileTemplates atIndexes:(NSIndexSet*)indexes
{
    [tileTemplates insertObjects:someTileTemplates atIndexes:indexes];
}

- (void)removeTileTemplatesAtIndexes:(NSIndexSet*)indexes
{
    [tileTemplates removeObjectsAtIndexes:indexes];
}

//These are for later.
#pragma mark -
#pragma mark Private Accessors

#pragma mark -
#pragma mark Other

@end

(Правка: я внес предложенные изменения и прокомментировал, какие ответы обсуждают их, на случай, если кому-то понадобится узнать почему.)

Ответы [ 4 ]

5 голосов
/ 28 февраля 2010

Пожалуйста, не упоминайте self = [super init]

Итак, почему не вы делаете это?

То же самое касается initWithCoder:: вы должны использовать объект, возвращаемый [self init], не предполагая, что он инициализировал исходный объект.

- (void)dealloc
{
    [super dealloc];
    [tileTemplates release];
}

Как сказал Абизерн в своем комментарии, [super dealloc] должен стоять последним. В противном случае вы обращаетесь к переменной экземпляра освобожденного объекта.

- (IRTileTemplate*)copyWithZone:(NSZone*)zone

Тип возвращаемого значения здесь должен быть id, соответствующий типу возврата, объявленному протоколом NSCopying.

{
    IRTileset* copy = [IRTileset new];
    [copy insertTileTemplates:tileTemplates atIndexes:[NSIndexSet indexSetWithIndex:0]];
    [copy setUniqueID:uniqueID];

Вы вставляете ноль или более объектов в один индекс. Создайте свой индексный набор с диапазоном: location = 0, length = счетчик массива tileTemplates. А еще лучше, просто присвойте значение всего свойства:

copy.tileTemplates = self.tileTemplates;

Или получить доступ к переменным экземпляра напрямую:

copy->tileTemplates = [tileTemplates copy];

(Обратите внимание, что вы должны сделать copy самостоятельно, обходя методы доступа к свойству, и что вы copy извлекаете массив от имени копии.)

    return [copy autorelease];
}

copyWithZone: не должен возвращать автоматически освобожденный объект. В соответствии с правилами управления памятью , вызывающая сторона copy или copyWithZone: владеет копией, что означает, что задача вызывающей стороны - выпустить ее, а не copyWithZone:.

@synthesize tileTemplates;
[et al]

Возможно, вы также захотите реализовать средства доступа к массиву из одного объекта:

- (void) insertObjectInTileTemplates:(IRTileTemplate *)template atIndex:(NSUInteger)idx;
- (void) removeObjectFromTileTemplatesAtIndex:(NSUInteger)idx;

Это необязательно, конечно.

2 голосов
/ 28 февраля 2010

// Однако я должен перечислить протоколы здесь, хотя они уже перечислены в IRTileset.h?

Нет, ты не должен. Расширение класса, объявленное в файле реализации, имеет расширение , поэтому вам не нужно заботиться о том, каким протоколам объявлен класс.

Я бы рекомендовал пометить имена переменных вашего экземпляра подчеркиванием: _tileTemplates. (Пуристы скажут вам ставить вместо подчеркивания префикс, а делать это, если вы их боитесь.)

Не используйте new для создания экземпляров классов. Насколько я понимаю, это никогда не рекомендуется.

[NSMutableArray new];                     //  :(
[NSMutableArray arrayWithCapacity:20];    //  :)

Не звоните [super dealloc], прежде чем делать свое собственное освобождение! Это может вызвать сбой при определенных обстоятельствах.

- (void)dealloc
{
    [tileTemplates release];
    [super dealloc];          // Do this last
}

Я не уверен, какой тип uniqueID имеет, но не должен ли он также быть выпущен в dealloc?

Я бы никогда не поместил свои директивы @synthesize в середину файла (поместите их сразу под «@ реализация»).

Также, не имея четкого представления о роли этого класса, countOfTileTemplates не звучит мне хорошо. Может быть, просто «count» сделает, если однозначно, что этот класс делает для хранения шаблонов плиток?

2 голосов
/ 28 февраля 2010

Это выглядит довольно хорошо, за исключением того, что вы оставили свои свойства открытыми для произвольных манипуляций внешними объектами В идеале данные должны обрабатываться непосредственно только самим классом модели, а внешние объекты должны иметь доступ только через специальные методы.

Например, что если какой-то внешний код вызывает это:

myIRTileset.tileTemplates=someArray;

Бум, ты потерял все свои данные.

Вы должны определить оба свойства данных только для чтения. Затем напишите методы доступа, внутренние для класса, которые будут управлять их сохранением в реализации класса. Таким образом, единственный способ для внешнего объекта изменить tileTemplates - это вызвать методы - insertTileTemplates:atIndexes: и removeTileTemplatesAtIndexes:.

Edit01:

Я думаю, что исказил это с первого хода, поэтому позвольте мне попробовать еще раз. Вам следует настроить класс модели данных по следующей схеме:

Интерфейс

@interface PrivateTest : NSObject {
@private 
    //iVar is invisible outside the class, even its subclasses
    NSString *privateString; 
@public
    //iVar is visible and settable to every object. 
    NSString *publicString; 
}
@property(nonatomic, retain)  NSString *publicString; //property accessors are visible, settable and getable. 
//These methods control logical operations on the private iVar.
- (void) setPrivateToPublic;  
- (NSString *) returnPrivateString;
@end

Таким образом, при использовании это будет выглядеть так:

Осуществление

#import "PrivateTest.h"

//private class extension category defines 
// the propert setters and getters 
// internal to the class
@interface PrivateTest ()
@property(nonatomic, retain)  NSString *privateString;
@end

@implementation PrivateTest
//normal synthesize directives
@synthesize privateString; 
@synthesize publicString;

// Methods that control access to private
- (void) setPrivateToPublic{
    //Here we do a contrived validation test 
    if (self.privateString != nil) {
        self.privateString=self.publicString;
    }
}

- (NSString *) returnPrivateString{
    return self.privateString;
}

@end

Вы бы использовали это так:

PrivateTest *pt=[[PrivateTest alloc] init];
    // If you try to set private directly as in the next line
    // the complier throws and error
//pt.privateString=@"Bob"; ==> "object cannot be set - either readonly property or no setter found"
pt.publicString=@"Steve";
[pt setPrivateToPublic];
NSLog(@"private=%@",[pt returnPrivateString]); //==> "Steve"

Теперь у класса есть пуленепробиваемая целостность данных. Любой объект в вашем приложении может установить и получить строковое свойство publicString, но ни один внешний объект не может установить или получить private.

Это означает, что вы можете безопасно разрешить доступ к экземпляру для любого объекта в вашем приложении, не беспокоясь о том, что небрежная строка кода в каком-то второстепенном объекте или методе все испортит.

0 голосов
/ 28 февраля 2010

два второстепенных придира: один метод init (где стилистически я против того, чтобы иметь 2 разных точки возврата, но это только я), однако ничто не мешает супер инициату возвратить другой объект, чем он сам или ноль, например другойобъект своего класса или даже просто еще один объект.По этой причине self = [super init], как правило, хорошая идея, даже если на практике это вряд ли что-то даст.Во-вторых, в методе copyWithZone вы не копируете tileTemplates, что может быть намеренным, но, как правило, плохой идеей (если только они не являются неизменяемыми).Копирование объекта должно иметь тот же эффект, что и выделение нового, например.сохранить счет 1, так что не выпускайте его автоматически.Кроме того, похоже, что вы ничего не делаете с зоной, поэтому вам, вероятно, следует заменить ее на что-то вроде

- (IRTileTemplate*)copyWithZone:(NSZone*)zone {
    IRTileset* copy = [[IRTileset allocWithZone:zone] init];
    [copy insertTileTemplates:[tileTemplates copyWithZone:zone]
                    atIndexes:[NSIndexSet indexSetWithIndex:0]];
    [copy setUniqueID:uniqueID];
    return copy;
}

, вот и все, что я нашел;за исключением количества сохраняемых копий (которое в дальнейшем приведет к ошибкам), в основном просто из того, что я предпочитаю, вы можете делать это по-своему, если вам это нравится больше.Хорошая работа

...