http://www.perlmonks.org?node_id=1016970

Lady_Aleena has asked for the wisdom of the Perl Monks concerning the following question:

Before I get too set in my (bad?) ways with writing objects, would some of you eyeball this and let me know where I veered (very?) off course. Basically a sanity check please?

package Twitter::Objects; use strict; use warnings FATAL => qw( all ); use lib '..'; use Base::Data qw(data_file get_hash); use Base::Convert qw(hashtagify); use Twitter qw(target_new_subscribers); use Twitter::ListSort qw(list_sortflags list_compare); # The 'data_file' subroutine takes 2 parameters (directory,filename). # The directory is the location within the main data directory. my %headings = ( account_totals => [qw(screen_name followers friends updates)], people => [qw(id screen_name name time_zone location last_status f +riends followers greet)], lists => [qw(id slug name user members_count members_change subsc +ribers_count subscribers_change status)], mentions => [qw(id screen_name date text reply_screen_name reply_id) +], retweets => [qw(id tweet retweeters)] ); # 'accounts' is to return an array(ref) of all of my Twitter accounts. sub accounts { my ($class) = @_; my @accounts = qw( Lady_Aleena LadyAleena_ABC LadyAleena_CBS LadyAleena_FOX LadyAleena_NBC LadyAleena_SyFy LadyAleena_TNT LadyAleena_USA LadyAleena_TV LadyAleena_eros LadyAleena_home LadyAleena_test ); return bless \@accounts; } # 'account_totals' is to return a hash(ref) totals such as # followers, friends, & updates each account has. sub account_totals { my ($class, $account) = @_; my %accounts = get_hash( file => data_file('Twitter','account_totals.txt'), headings => $headings{account_totals}, ); return bless $accounts{$account}; } # 'acct_data' returns other collected data for my Twitter # accounts. See the headings for people, mentions, and # retweets for the data to be returned as a hash(ref). sub acct_data { my ($class, $account, $type) = @_; die "Invalid type, $type does not exist." if !$headings{$type}; my %data = get_hash( file => data_file("Twitter/users/$account","$type.txt"), headings => $headings{$type}, ); return bless \%data; } my $base_link = "https://twitter.com/#!"; my %add_lists_data = ( 'Advtr of Brisco County Jr' => { long_name => 'The Adventures of Brisco County, Jr.', abbr => 'Bricso County' }, 'Buck Rogers 25th Century' => { long_name => 'Buck Rogers in the 25th Century', abbr => 'Buck Rogers' }, 'Crossing Jordan/Las Vegas' => { long_name => 'Crossing Jordan & Las Vegas', abbr => 'CJLV' }, 'ER Third Watch Med Invgtn' => { long_name => 'ER, Third Watch, & Medical Investigation', abbr => 'ETM' }, 'Eureka Warehouse13 Alphas' => { long_name => 'Eureka, Warehouse 13, & Alphas', abbr => 'EWA' } ); # 'lists_data' returns a hash(ref) of all the basic data # collected about my lists (owned, subscribed to, and a member of # on Twitter. 'lists_data' also collates other data about the lists # I own such as how many more subscribers I want for each list. Also # it puts the list members and subscribers into arrays. Some lists # have a few special options, so the %add_lists_data is included into # the hash(ref) returned for those lists. sub lists_data { my ($class, $account) = @_; my %lists = get_hash( file => data_file("Twitter/users/$account","lists.txt"), headings => $headings{lists}, ); for my $list (values %lists) { list_sortflags($list); my $user = $list->{user}; my $slug = $list->{slug}; my $name = $list->{name}; my $mems = $list->{members_count}; my $subs = $list->{subscribers_count}; my $abbr = $add_lists_data{$name}{abbr} ? $add_lists_data{$name}{a +bbr} : ''; $list->{abbr} = $abbr; $list->{hashtag} = length($abbr) ? hashtagify($abbr) : hashtagif +y($name); $list->{link} = "$base_link/$user/$slug"; $list->{long_name} = $add_lists_data{$name}{long_name} ? $add_list +s_data{$name}{long_name} : $name; $list->{percent} = int(($subs / $mems) * 100) if $mems > 0; $list->{target} = target_new_subscribers($list->{members_count} +,$list->{subscribers_count}); my @statuses = split(/,/,$list->{status}); $list->{status} = [@statuses]; next if !grep(/owner/,@{$list->{status}}); for my $list_people (qw(members subscribers)) { my $file = data_file("Twitter/users/$account/lists/$slug","$list +_people.txt"); next unless -s $file; open(my $lp_fh,'<',$file); my @lp = <$lp_fh>; chomp(@lp); $list->{$list_people} = [@lp]; } } return bless \%lists; } # 'all_lists' takes all of the lists from all of my accounts # and puts them into one big hash(ref). sub all_lists { my @accounts = Twitter::twitter_accounts; my %lists; for my $account (@accounts) { my $sublists = Twitter::Objects->lists_data($account); for (keys %{$sublists}) { $lists{$_} = $$sublists{$_} unless $lists{$_}; } } return bless \%lists; } # 'lists_people' is a hash(ref) which is all of the members # and subscribers of all my lists so I know who is a member or # subscriber to more than one list. sub lists_people { my $lists = Twitter::Objects->all_lists; my %people; for my $list (sort { list_compare($a,$b) } values %{$lists}) { my $name = $list->{name}; next if !grep(/owner/,@{$list->{status}}); for my $type (qw(subscribers members)) { if ($list->{$type}) { my @list_people = @{$list->{$type}}; for my $person (@list_people) { push @{$people{$type}{$person}}, $name; } } } } return bless \%people; } 1;

Update: per mbethke's suggestion, I added comments to let you know what each object is supposed to do. I hope they are clear enough.

Have a cookie and a very nice day!
Lady Aleena

Replies are listed 'Best First'.
Re: RFC on how I'm doing when it comes to writing objects?
by mbethke (Hermit) on Feb 04, 2013 at 17:04 UTC

    Hi LadyAleena,

    three things that caught my attention:

    1. Some comments would be nice, otherwise it's hard to deduce what the individual methods are supposed to do without seeing the data and other classes you're using.
    2. Always use 2-argument bless, i.e. bless $ref, $class, otherwise subclassing will not work should you decide to do it later.
    3. The whole blessing ritual doesn't actually gain you anything here as your methods as basically all class methods that you use like glorified procedures. The point of having objects (well, one of them) is to allow for more flexibility with regard to things you have hardcoded right now like the data file name and stuff like %add_lists_data without having to specify them each time. E.g.
      sub new { my ($class, $datafile, $accounts) = @_; return bless { file => $datafile, accounts => $accounts, }, $class; }
      Then you can call methods on that object an reuse the data you passed in on new(), and e.g. cache more data in your instance data as you read them (in case you're likely to need them again later):
      sub acct_data { my ($self, $type) = @_; die "Invalid type, $type does not exist." unless defined $headings +{$type}; return $self->{data}{$type} if defined $self->{data}; return $self->{data}{$type} = { get_hash( file => data_file($self->{file}, "$type.txt"), headings => $headings{$type}, ) } }

      Comments are in. Hope you find them helpful in pointing me in the right direction. I tested the 2 argument bless in the first subroutine. When it was data dumped, I didn't see any change in how the data was stored.

      I went with the first one since it is the most straight forward. It does not have any of my other custom subroutines used in it and stands on its own.

      sub accounts { my ($class) = @_; my @accounts = qw( Lady_Aleena LadyAleena_ABC LadyAleena_CBS LadyAleena_FOX LadyAleena_NBC LadyAleena_SyFy LadyAleena_TNT LadyAleena_USA LadyAleena_TV LadyAleena_eros LadyAleena_home LadyAleena_test ); return bless \@accounts, $class; }

      After adding $class on the last line, the data dumped looked like...

      $VAR1 = bless( [ 'Lady_Aleena', 'LadyAleena_ABC', 'LadyAleena_CBS', 'LadyAleena_FOX', 'LadyAleena_NBC', 'LadyAleena_SyFy', 'LadyAleena_TNT', 'LadyAleena_USA', 'LadyAleena_TV', 'LadyAleena_eros', 'LadyAleena_home', 'LadyAleena_test' ], 'Twitter::Objects' );

      I got the same result without $class on the last line. When I use the subroutine as an object, I would do something like...

      use strict; use warnings; use lib '../files/perl/lib'; use Twitter::Objects; my $accounts = Twitter::Objects->accounts; for my $account (@{$accounts}) { print $account; # and other code I want to run through this loop. }

      I'm not sure what I'm missing.

      Have a cookie and a very nice day!
      Lady Aleena
        Comments are in. Hope you find them helpful in pointing me in the right direction. I tested the 2 argument bless in the first subroutine. When it was data dumped, I didn't see any change in how the data was stored.

        It makes no difference now, nor would it matter with the way you use the returned values. But it will cause you problems later once you start having a proper class hierarchy because it breaks inheritance. Just forget the single-argument blessexists, it's no good.

        I got the same result without $class on the last line. When I use the subroutine as an object, I would do something like...
        my $accounts = Twitter::Objects->accounts; for my $account (@{$accounts}) { ... }

        That's not using it as an object, that's a plain old arrayref that works the same whether or not you bless it.

        As 7stud and others have pointed out, the basic problem is that you haven't organized your code in an OO way so "objectifying" it doesn't work like that. You first have to identify what the "things" are in the system you're building, and what properties they have. "Accounts" would probably be such a thing---a collection of account objects. You'd pass its constructor, "new() usually, a directory that it reads to determine what the accounts are called. If stuff lives in Twitter/$account/... you can generalize that and don't have to hardcode the list. Remember the whole OO dance is supposed to help with code reuse and if the code contains your account list it's not reusable at all. So the instance data would be e.g. a directory and a list of names. Then you could ask the Accounts collection for a list of account names and for an object representing a named account. The Account class wouldn't have to know that its configuration lives in a certain directory hierarchy, it just gets passed a directory by the Accounts collection and reads its files from there. "Lists" would probably be a class of its own that somehow deals with an Accounts object, though I don't understand well enough what a "list" is in this context.

        Playing with the code from perlboot should give you a better understanding of how to change your code organization so it actually benefits from OOP.

        To expand on what mbethke said:

        Think of each of your accounts as an entity that is self (pun intended) contained - It has information about that account stored in it and has methods that know what to do with that information.

        Each of these entities could be an object, the template for which is defined by the class that you write like so:

        package My::Twitter::Account; use strict ; use warnings ; .... sub new { my $class = shift; my %parameter_hash; my $count = @_; %parameter_hash = @_; croak( "MISSING FILE : \n " . $useage_howto ) unless( $ +parameter_hash{ FILE } ) ; croak( "MISSING HEADINGS: \n " . $useage_howto ) unless( $ +parameter_hash{ HEADINGS } ) ; croak( "MISSING NAME : \n " . $useage_howto ) unless( $ +parameter_hash{ NAME } ) ; $parameter_hash{ DEBUG } = 0 unless( $parameter_hash{ DEBUG } ); my $self = { FILE => $parameter_hash{ FILE } , HERADINGS => $parameter_hash{ HEADINGS } , NAME => $parameter_hash{ NAME } , SOME_USEFUL_INFO_MAYBE => $other_useful_info , DEBUG => $parameter_hash{ DEBUG } , } bless( $self, $class ); dump( $self ) if( $self->{DEBUG} == 1 ); return $self; } ... Other Methods that deal with a SINGLE twitter account here.

        Now that we have something to deal with a single account we can combine these either into a second package or decide to use them in a non-OO way. Lets do the First:

        package My::Twitter::AllAccounts; use strict ; use warnings ; ... blah blah blah sub new { my $class = shift; my %parameter_hash; $parameter_hash{ DEBUG } = 0 unless( $parameter_hash{ DEBUG } ); my @twitter_accounts ; my $self = { TWITTER_ACCOUNTS => \@twitter_accounts , DEBUG => $parameter_hash{ DEBUG } , } bless( $self, $class ); dump( $self ) if( $self->{DEBUG} == 1 ); return $self; } sub add_twitter_account { my $self = shift ; my $name = shift ; my ( $file, $headings ) = _function_that_gets_files_and_headi +ngs(); # This can also be inside the Twitter::Account class. my $twitter_account = new My::Twitter::Account( NAME => $name , FILE => $file , HEADINGS => $headings , ); push( @{ $self->{ TWITTER_ACCOUNTS } }, $twitter_account ) ; return 1; }

        Now we have a way to store a single twitter account and a way to store the collection we can do things like:

        my $twitter_data = new My::Twitter::AllAccounts() ; my @accounts = qw( Lady_Aleena LadyAleena_ABC LadyAleena_CBS LadyAleena_FOX LadyAleena_NBC LadyAleena_SyFy LadyAleena_TNT LadyAleena_USA LadyAleena_TV LadyAleena_eros LadyAleena_home LadyAleena_test ); foreach my $account ( @accounts ) { $twitter_data->add_twitter_account( $account ); }

        Of course the really fun part begins now:

        # Am assuming you move your functions into the correct classes and wri +te some ... But the below is just for illustration. $twitter_data->reload_account_data( $account_name ); $twitter_data->get_sum_of_all_accounts(); $twitter_data->remove_twitter_account( $account ); $twitter_data->temporarily_ignore_accounts( [ account1, account2 ] ); $twitter_data->get_sum_of_all_accounts(); $twitter_data->stop_ignoring();
Re: RFC on how I'm doing when it comes to writing objects?
by tobyink (Canon) on Feb 04, 2013 at 18:53 UTC

    Calling "bless" randomly in your functions does not make your code OO, any more than saying "le" occasionally confers French citizenship upon you.

    package Cow { use Moo; has name => (is => 'lazy', default => sub { 'Mooington' }) } say Cow->new->name

      As far as I knew, when I return a blessed reference to data in a subroutine, I have an object which I do not have to export. I put the bless where I was told to put it in each subroutine.

      Have a cookie and a very nice day!
      Lady Aleena

        "As far as I knew, when I return a blessed reference to data in a subroutine, I have an object which I do not have to export."

        As far as I knew, "le" is a proper French word, meaning "the".

        I don't dispute that your code returns blessed objects. There is more to that than writing object-oriented code though. As per your update:

        "I added comments to let you know what each object is supposed to do"

        That alone is a warning sign. If the different objects do different things, then they should be instances of separate classes.

        Your "objects" apparently don't have any object methods, so why are you using "objects" rather than plain old hashes?

        package Cow { use Moo; has name => (is => 'lazy', default => sub { 'Mooington' }) } say Cow->new->name
Re: RFC on how I'm doing when it comes to writing objects?
by 7stud (Deacon) on Feb 05, 2013 at 01:09 UTC

    Another bad sign is that you called your class 'Objects' (albeit in the directory Twitter). First of all, class names are singular, e.g. Dog, Cat, DBConnection, etc. If you need to create many objects of your class, then you can store them in an array, perhaps named @dogs, @cats, or @db_connections. Or maybe you want to store your objects in a custom container created from one of your classes named MySpecialContainer--again a singular class name.

    Of course, you could quickly change your class name to the singular Object, but then the problem is: you would never name a class Object--because it's non descriptive; it's like naming a variable in your program $var. When there is no obvious name for your class like Dog, Cat, or DBConnection, that might be an indication that you don't need a class at all. If all you want to do is create a bunch of useful functions, then put them in a module called Twitter::MyTwitterFunctions, and forget about classes.

    If you do want to stay the course with classes, your code does lend itself to creating a class named Account (or Twitter::Account). Start by defining a new() method that takes arguments such as the account name, password, etc., then think of things you would like to do with an account: tweet() a message? get_total_followers(), etc.? Then write a program that creates an Account object for each of your accounts (maybe reading each account name, password, etc. from a text file), then loop through the Account objects to get the information from each Account object and calculate the totals you want.

    You can certainly spin off the work that needs to be done to subroutines, so your final program might look something like this:

    use strict; use warnings; use 5.012; use Readonly; use Account; Readonly $SPACE => q{ }; #A single space my @accounts; for my $acct_info ($ACCT_FILE) { push @accounts, Account->new(split $SPACE, $acct_info); } say get_total_followers(@accounts); #etc. #etc.

    You could take things a step further and create an AccountsManager class. Its new() method could take an array of Account objects as an argument, and the methods you define in the AccountsManager class could return totals for all the Account objects. You could also define add() and remove() methods for the class to allow you to change which accounts are being managed.

      I have been shown I should get away from the idea of objects for this module since I am producing arrays and hashes instead of other things making this module as it is silly. All of the Twitter functionality is handled quite well in Net::Twitter::Lite. I do not want to recreate the wheel. Thank you for helping me see where I went wrong.

      Have a cookie and a very nice day!
      Lady Aleena

      you would never name a class Object

      Hey. A lot of programming languages have a class called Object. Are you suggesting the designers were wrong? :)

        Hey. A lot of programming languages have a class called Object. Are you suggesting the designers were wrong? :)

        I've used at least one of them: ruby. Yes, this:

        @you = grep { ! $_->is_oo_runtime_designer } @everybody

        In the limit, in Smalltalk, everything is an object...:)

        James

        There's never enough time to do it right, but always enough time to do it over...

        you would never name a class Object
        Hey. A lot of programming languages have a class called Object. Are you suggesting the designers were wrong? :)

        That should be read as "@youš would never ..."

        @you = grep { ! $_->is_oo_runtime_designer } @everybody

Re: RFC on how I'm doing when it comes to writing objects?
by jmlynesjr (Deacon) on Feb 04, 2013 at 17:20 UTC

    I am working on learning to write objects myself. The following is a draft document I am writing to document what I am finding in the documentation and in posts and emails. It's related to wxPerl, but maybe it will be of use to you as well. Also,take a look at my wxPerl LCD Clock post to see the direction I am going at the moment.

    Update: Added link to clock post.

    James

    There's never enough time to do it right, but always enough time to do it over...