3
\$\begingroup\$

I have two classes DigBackground and DigParallaxBackgroundLayer (a parallax background is a image consisting of several images that move at different scroll speeds, for games etc.). My idea was to have the DigBackground class which is basically just a image with some added properties that can be set were it will draw itself on the screen. It just has one method currently and that is the initWithImageNamed that sets all of it properties. Some of its properties are public, but read-only. So they can be accessed by the DigParallaxBackground class.

The DigParallaxBackground class currently also have one method at present - the initializer which takes an array of DigBackground´s as argument. And will display them on screen.

I want to know if this is decent design and how I can improve it if need be.

This is how it's used:

    DigBackground *bg1 = [[DigBackground alloc]initWithImageNamed:@"Default.png"
                                                  withScrollSpeed:1.0f
                                                  InitialOffsetOf:ccp(0, 0)
                                                         position:ccp(100, 100)
                                                         setFlipX:NO];




    DigParallaxBackgroundLayer *parallaxBg = [[DigParallaxBackgroundLayer alloc]
                                              initParallaxBackgroundWithImages:@[bg1] inOrder:kAscending];


    [self addChild: parallaxBg];

Header for DigBackground:

@property (nonatomic, readonly) float bgScrollSpeed;
@property (nonatomic, readonly) CGPoint initialOffset;


// Designated initializer
- (id) initWithImageNamed: (NSString *)image
          withScrollSpeed: (float)scrollspeed
          InitialOffsetOf: (CGPoint)offset
                 position: (CGPoint)position
                 setFlipX: (BOOL)flipX;

Header for DigParallaxBackground:

typedef enum {
    kAscending,
    kDescending
} ImagesOrder;

@interface DigParallaxBackgroundLayer : CCLayer
{
    NSArray *backgrounds; 
}


// Designated initializer
- (id) initParallaxBackgroundWithImages: (NSArray *) images inOrder: (ImagesOrder) order;
\$\endgroup\$
  • 1
    \$\begingroup\$ Looks pretty straightforward. InitialOffsetOf: is not idiomatic, I would have expected lowercase, and why not just initialOffset:? Also, are there technical reasons why the properties should be immutable once set? I would think that especially in games, being able to vary the scroll speed is an advantage if implementing that is straightforward. \$\endgroup\$ – microtherion Jun 5 '13 at 12:54
5
\$\begingroup\$

Just a couple nit-picky things...

InitialOffsetOf: should instead be initialOffset:

withScrollSpeed: should just be scrollSpeed:

I'd probably also add factory methods. When I'm writing code, no class is complete until it has its factory methods.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy