It comes with practice and experience. It comes easy for me because I’ve coded professionally for decades and my experience on this forum helping people with Rules has given me a whole lot of experience and practice.
The key thing is when you find yourself typing the same line(s) of code over and over with only minor changes that is considered a violation of the DRY principal. Don’t Repeat Yourself.
I looked at your example above and I see that you are repeating yourself with the Rule triggers -> Move to Member of trigger.
I looked at your Rule body and saw essentially the same if statement and sendNotification line over and over. So my coder spidey sense goes off and I ask, how can I keep from repeating these lines over and over. Well, the first thing I can do is only call sendNotification once in the Rule at the bottom. That clears out that repeated line. So what is different in the repeated calls to sendNotification? It’s the message so before I get to the Rule I need to calculate the message to send.
So far so good. The Rule would look something like this:
rule "Motion Detection (PART)"
when
Member of MotionSensors changed from OFF to ON
then
// blah blah bla
val message = ???
sendNotification("myemail@live.com", message)
end
So now I need to focus on how to calculate the message. I look for patterns and I see the the message is just the Item name using spaces instead of underscores with " Detected!" appended to it. That makes it easy. I just need to do some work with triggeringItem’s name and I can create the message in one line.
Often this isn’t the case and this middle part of the Rule would be a bit more complicated.
So the Rule becomes:
rule "Motion Detection (PART)"
when
Member of MotionSensors changed from OFF to ON
then
// blah blah bla
val message = triggeringItem.name.replace("_", " ") + " Detected!"
sendNotification("myemail@live.com", message)
end
Phew, no more duplicated code. Now we need to look at the rest of the Rule. All that is left is to check if Security_System_PART.state is ON and not run the Rule if it is not. I could put the rest of the Rule inside an if statement, but I find Rules with fewer indents and curly brackets are simpler and easier to follow, so we will flip the if around and exit the Rule if that Item is anything but ON.
And we end up with the Rule I posted above.
I can’t speak for Vincent’s thought processes, but here is how I would go about thinking about your OP Rules.
I see basically the same Rule repeated over and over doing the same thing on different Items. My DRY spidey sense is going off. Let’s look for the patterns in what is different between the Rules.
- Rules are all triggered by a Motion sensor Item.
- Each Rule has an associated Item. I.e. each motion sensor has an associated LastTriggered Item and the names are almost such that I can take the name of the triggeringItem and append “LastTriggered” to it and will have the name of the associated Item.
- When the Motion sensor changes we update the associated Item with it’s last update.
Something else comes to mind which is a result of experience and I wouldn’t expect most users to think of this at first. The Rules are going back to persistence to get the lastUpdate value that is being sent to the LastTriggered Item. Unfortunately, persistence may not be done saving the most recent value to the database when this Rule runs so you can’t really rely on lastUpdate without a small sleep (100-200 msec) is usually enough. But this Rule gets triggered from that most recent update anyway so we can avoid the call to Persistence and the risk that we get the wrong value and instead just use now.
Therefore the recommendation would be:
- Merge them all into one Rule.
- Use a Group and Member of to trigger this merged Rule
- Rename the LastTriggered Items so the first part matches the Motion sensor Item names
- postUpdate using now
Resulting in:
rule "Kitchen Motion last Updated"
when
Member of Member of MotionSensors changed
then
postUpdate(triggeringItem.name+"LastTriggered", now.toString)
end
Now I notice that you have two Rules that trigger when those MotionSensors go off. They could be merged into one Rule.
rule "Motion Detection (PART)"
when
Member of MotionSensors changed
then
postUpdate(triggeringItem.name+"LastTriggered", now.toString)
if(Security_System_PART.state != ON || previousState != OFF || triggeringItem.state != ON) return;
val message = triggeringItem.name.replace("_", " ") + " Detected!" // replace the underscores with spaces in the Item name an append the word "Detected!" as the message.
sendNotification("myemail@live.com", message) // send the message
end
So what changed? I removed the from part of the Rule trigger. I want this Rule to trigger on any change.
I added a new line to update the LastTriggered Item with now.
Then I added some more checks to replace the from OFF to ON part that I removed from the Rule trigger so the notification will only be sent in the case where the sensor went from OFF to ON.
That is how I think about code like you posted and come up with how to improve it. That is how I (and probably others like Vincent too) can take 83 lines of code scattered across 9 Rules and collapse them down to a single Rule of 12 lines of code. You too will get to this point and probably faster than you realize. None of us, even the professional coders, start out writing these super concise and efficient Rules. It took me well over a year of helping people on this forum and applying little tricks that I’ve learned to halve the number of lines in my Rule, then halve them again.
And when I discover some new problem I’ve not encountered before or some new way to improve the code, I write a DP to document it. That is why often a response to questions like this will sometimes just be a link to a DP. I and the others who have written DPs try to do so with lots and lots of description and documentation, much more than could be included in a response to a question.
Over time you will start to think about Rules in this way more and more. You have already taken the first step. Your spidey sense is going off. Otherwise you wouldn’t have posted here in the first place.
but i haven’t got a clue how that works its complex through simplicity
i will let you know how small my system is
The only long file is the dexcom one, and that is because it would properly be implemented in a binding anyway and once I figured out the data is useless I never went back to edit the code down. As with writing, the first draft of a piece of code is rarely any good.