The application (client, server, and module) are all available on my personal Mediaone webpage. The client is here. The server is here. Lastly, the module is here. I'm trying to make the module more general, to handle different datasets and such, since the info file I'm using may not be the same one I'm using a year from now. So, I was looking for possible ideas to expand the usage of my module so that it would be more general (and possibly even make it to CPAN for it's small niche). The SIS system is a very strange beast, and I'm trying to make this code very portable (so that others may use it, even though they have a different info file).
This "info file" I'm talking about is a flat text file (comma delimited) that has data about a student. Most of the time it includes the name, homeroom, track, homeroom number, homeroom teacher, and student number (some sequential number from 1-9999). The data can be arranged any way the programmer wants. Since I really do not know HOW the data will be stored in that flat text file, I'd like to be able to handle any orientation of data (I know I'll still have to analyze the file to see how the data is arranged, but I would like to be able to handle any case just by passing a different set of parameters to my subs.) I'm still somewhat new to application development, so I'm trying to incorporate scalability into my code. That way, should I get a new info file with differently arranged data, I won't have to touch my module. I can just send it a different set of parameters (to tell the sub how the data is arranged) and go from there.
It may just be that I'm trying to do more than I need to with this module idea, but I think making the subs I'm using modular would make things easier for others in my situation. Again, any suggestions would be helpful.
Theodore Charles III
Network Administrator
Los Angeles Senior High
4650 W. Olympic Blvd.
Los Angeles, CA 90019
323-937-3210 ext. 224
email->secon_kun@hotmail.com | [reply] |
Sorry, I don't have time to address this in detail any time soon. Just a few specific criticisms on points you may be missing:
- OO modules generally should not use Exporter. The correct class that calls are to be dispatched to should be decided by the syntax of the method call.
- No strict.pm. I can tell from your variable usage that you have the good habits it is meant to encourage. However it is a lexically scoped declaration and needs to be in a module for you to get the typo checks. (I list this after Exporter because I like putting Exporter related stuff before I use strict.
- Your constructor is designed to be called as a function. This is probably connected to your exporting it. But that isn't good. You are overwriting the generic function name new() in other namespaces. Here is a much better constructor:
sub new {
my ($class, %self) = @_;
return bless \%self, $class;
}
- You are using full tabs for indents. It is important that indents be 2-4 spaces. You might be solving this by using custom tabstops. This helps you, but custom tabstops that differ between people cause their own potential problems (try programming in Python to find out what). I prefer using explicit spaces. (Easy with any decent programming editor.)
- You have a lot of Student::get_attr() calls which are bad for two reasons. The first is that you are in package Student, if you meant to write them as function calls they are better written as get_attr(). The second is that they are actually all method calls. To move from doing OO lipservice to taking advantage of what OO does, it is important to switch syntax to $obj->get_attr(...), it isn't just that the syntax is "right", it is that you now do a method dispatch on type without scattering the assumption about type in your code.
- You localize $_ unnecessarily. Having been bitten by something like this before, I sympathize with paranoia. But just in case you didn't know, Dominus's tutorial Coping with Scoping mentions the autolocalization in loops.
- You have certain code which you write again and again. If you find yourself doing that (or cutting and pasting), that is a sign that you really wanted a function. For instance you write the same 5 lines over and over again for a random letter. Don't. Write a 5-line function and call it.
- On a related note, you don't appear to have yet made the jump from, "this is how I will accomplish a certain kind of task" to saying, "I am going to factor that out by writing an interface to accomplishing this task". That is how you should handle your logging issue. Write an interface to sending log messages, and then call that interface. Decide later whether that goes to a file, STDERR, or whatever.
OK, I know that is a lot to digest, but I am going to give you one more item. It isn't Perl specific, but Code Complete is an excellent book which I think would be an excellent next step for you. It is mainly about procedural programming, yes, but the things it teaches are generally applicable, and will give you enough foundation in the principles of good procedural design to see what problems OO addresses.
| [reply]
[d/l]
[select] |
Thanks for the constructive criticism tilly. I really appreciate it. That's the kind of advise I was looking for when I wrote this node. I'm still new to OO programming, so I'm expecting to get my toes stepped on before I have a decent set of code. As far as the Exporter, I was assuming that the particular user of the module may or may not use the methods as methods, but as function calls. I guess the easier way to handle the dual usage (which I believe is done in Tk) is to just write code that can handle both (of course, being somewhat new to OO, that might be a little easier said than done). With every little piece of advise, my code just gets that much better.
One of the problems I was having with using method calls instead of function calls was that for some reason or another, when the $client->rpc($obj->func($params)) is done, it seems to miss the method altogether. I guess this may be a bug in the RPC module that I'm using (reference: Advanced Perl Programming). I'll have to play with that some more.
As far as the logging procedure goes, I'm stuck between two methods:
a.) Trying to flush to a log file as fast as possible (since I will have to check against this log to see if a particular user exists)
b.) Holding the log for a period of time in ram and then flushing after a set amount of time (I'm still not sure if this is even possible).
I understand what you're saying that I'm not thinking of an interface (in terms of logging), but in this case, I have to think of reading and writing the log within seconds of each other. Hmmm... I just had an idea. Is it possible to block while the log file is in read or write mode? In other words, if one user has called the 'usr_create' method, is it possible to block all other calls while the first one is finishing? I'm still somewhat new to the IPC/RPC idea, so I may just be guessing madness. If so, the interface for logging would be a little easier to implement (since I could force the other clients to wait just a second while I read and write to the log file).
Theodore Charles III
Network Administrator
Los Angeles Senior High
4650 W. Olympic Blvd.
Los Angeles, CA 90019
323-937-3210 ext. 224
email->secon_kun@hotmail.com
| [reply] |