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
Re: RFC on how I'm doing when it comes to writing objects?
by mbethke (Hermit) on Feb 04, 2013 at 17:04 UTC
|
| [reply] [Watch: Dir/Any] [d/l] [select] |
|
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
| [reply] [Watch: Dir/Any] [d/l] [select] |
|
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.
| [reply] [Watch: Dir/Any] [d/l] [select] |
|
|
|
|
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();
| [reply] [Watch: Dir/Any] [d/l] [select] |
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
| [reply] [Watch: Dir/Any] |
|
| [reply] [Watch: Dir/Any] [d/l] |
|
"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
| [reply] [Watch: Dir/Any] |
|
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.
| [reply] [Watch: Dir/Any] [d/l] |
|
| [reply] [Watch: Dir/Any] |
|
| [reply] [Watch: Dir/Any] |
|
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
| [reply] [Watch: Dir/Any] [d/l] |
|
| [reply] [Watch: Dir/Any] |
|
| [reply] [Watch: Dir/Any] |
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...
| [reply] [Watch: Dir/Any] [d/l] |
|
|