NotionCommotion Posted June 5, 2020 Share Posted June 5, 2020 I know it is often frivolous, but sometimes I can't help trying to optimize things, and use the following two methods to create either an object injected with a string or one of a set of objects. For both cases, the class has no setters so there is no chance that the object will later be changed. I first stated doing this in continuously running server applications which I believe makes sense, however, it has leaked over to standard HTTP requests. Please commit good or bad on this approach. Thanks public static function create(string $unit):self { static $stack=[]; if(!isset($stack[$unit])) { $stack[$unit] = new self($unit); } return $stack[$unit]; } public static function create(string $class):FunctionInterface { static $stack=[]; if(!isset($stack[$class])) { $stack[$class] = new $class; } return $stack[$class]; } Quote Link to comment Share on other sites More sharing options...
requinix Posted June 5, 2020 Share Posted June 5, 2020 Do not optimize for memory unless you're running in a constrained environment. Which you aren't. PHP is perfectly capable of cleaning up unused variables. This design pattern is fine as long as you're using it for the right reason: because each $unit/$class should only ever be created once (in this context) and it's acceptable, if not desirable, to reuse instances in multiple places. Without knowing your usage, I would expect that it's better not to use a registry/singleton pattern like this, and that you should be creating and destroying data as needed. Also: that's not a stack so don't call it one. Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted June 5, 2020 Author Share Posted June 5, 2020 Would a socket server which runs continuously be considered a "constrained environment" or even then let PHP clean up unused variables? My first static create method accepts "hour", "day", "week", etc and creates a TimeUnit object which get's injected into each new Time object. After being constructed, a TimeUnit never changes and each "day TimeUnit" in every instance of Time is the same. Registry/singleton pattern or create/destroy each TimeUnit as needed? While it doesn't for my application, maybe only use a singleton if TimeUnit had some "usedByTheseTimes" property? Thanks for correcting my use of the word "stack". See your point. I will call it "registry" unless you think I shouldn't. Quote Link to comment Share on other sites More sharing options...
kicken Posted June 5, 2020 Share Posted June 5, 2020 37 minutes ago, NotionCommotion said: Would a socket server which runs continuously be considered a "constrained environment" or even then let PHP clean up unused variables? Just because it's running continuously doesn't make it constrained. Constrained has to do with limited resource availability, like needing to run your script on a small VM with only 1G ram for example. You could always impose your own artificial constraints, such as keeping your scripts capable of running with memory_limit=256MB, but you need to also decide if that's reasonable for the environment/task. In my experience that's usually fine for most web scripts. Being a continuously running script isn't going to affect PHP's ability to clean up memory either. The clean up is something that happens whenever PHP needs to do it. I don't know the finer details of when exactly PHP decides to run a GC cycle, but I tend to assume it does so when leaving a scope or when hitting an OOM condition trying to allocate memory for something. It's generally not relevant when they happen though, bottom line is that they do and the end result is your dead objects won't be hanging around eating up memory. 1 hour ago, NotionCommotion said: While it doesn't for my application, maybe only use a singleton if TimeUnit had some "usedByTheseTimes" property? Not entirely sure what you mean by that. You use a singleton when you only want (or need) one instance of a particular object, for example your database connection or session data. There's no requirement that that object some how link back to everything that's using it. TimeUnit to me sounds like it's probably a fairly small/easy to make object so I'd just do new TimeUnit('blah') whenever one was needed, similar to just doing a new DateTimezone('UTC') whenever I need one instead of trying to make one global instance. I reserve this singleton strategy for cases where either I truly want only one instance or the objects are large and complex to make (ie, require several database queries, some heavy processing, etc) and having one shared instance is fine. 56 minutes ago, NotionCommotion said: I will call it "registry" unless you think I shouldn't. I tend to call such a variable a $cache but registry probably is fine as well. If you're on PHP 7, you could shorten your code a bit: static $cache=[]; $cache[$class] = $cache[$class] ?? new $class; return $cache[$class]; Quote Link to comment Share on other sites More sharing options...
requinix Posted June 5, 2020 Share Posted June 5, 2020 +1 to $cache. Because that's what it is. Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted June 5, 2020 Author Share Posted June 5, 2020 47 minutes ago, kicken said: Not entirely sure what you mean by that. You use a singleton when you only want (or need) one instance of a particular object, for example your database connection or session data. There's no requirement that that object some how link back to everything that's using it. Thanks kicken, I've used this approach when I have various groups where objects are first assigned to a group and then processed. $grouper->assign($thing) would determine which group $thing belongs to based on $thing's properties and then get existing or create new $group and execute $thing->setGroup($group) , and Thing::setGroup($group) would execute $this->group=$group and $group->registrar($this), and group would all the thing to its collection. The point is $grouper needs to pass an existing Group object should it exist. Instead of TimeUnit::usedByTheseTimes, it would be Group::members or something. Yeah, I totally understand why you said "Not entirely sure what you mean by that". Yep, PHP7 and like the ?? operator. Just peeked at requinix's reply. Will go with $cache. Thanks both of you! Quote Link to comment Share on other sites More sharing options...
requinix Posted June 5, 2020 Share Posted June 5, 2020 Oh. About shortening code, it can go one step further: static $cache = []; return $cache[$class] ?? $cache[$class] = new $class; If you want to have a good time, figure out how that works. If you want to have a really good time, figure out how that works given that ?? has higher precedence than = Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted June 5, 2020 Author Share Posted June 5, 2020 3 hours ago, requinix said: Oh. About shortening code, it can go one step further: static $cache = []; return $cache[$class] ?? $cache[$class] = new $class; If you want to have a good time, figure out how that works. If you want to have a really good time, figure out how that works given that ?? has higher precedence than = It is not as readable as kicken's code but it isn't horrible. Doesn't it work specifically because ?? has higher precedence than = ? If $cache[$class] isn't NULL, return it, else return $cache[$class] which is equal to new $class. Quote Link to comment Share on other sites More sharing options...
requinix Posted June 5, 2020 Share Posted June 5, 2020 25 minutes ago, NotionCommotion said: Doesn't it work specifically because ?? has higher precedence than = ? If $cache[$class] isn't NULL, return it, else return $cache[$class] which is equal to new $class. Precedence is like automatic grouping with parentheses. You know how multiplication has higher precedence than addition? That's why 1 + 2 * 3 is 7 and not 9: because it's like 1 + (2 * 3). Since ?? has higher precedence, the statement seems like it should do return ($cache[$class] ?? $cache[$class]) = new $class; Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted June 6, 2020 Author Share Posted June 6, 2020 Yes, I see your point, but I interpreted it differently. While I couldn't find in the docs, [] has higher priority maybe. Regardless, the C girls and boys coded exactly how I wanted it to be. return is_null($cache[$class])?$cache[$class] = new $class:$cache[$class]; Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.