This topic contains 8 replies, has 3 voices, and was last updated by
February 10, 2019 at 11:39 pm #138934
Just playing around with tool making. I have a couple steps left to do to incorporate a couple of switch parameters I've included but, generally, with what's there so far, thoughts? What could be done smarter or cleaner? Would love any feedback you can share!
Thanks for anything you can share!
February 11, 2019 at 12:54 am #138948ParticipantTopics: 3Replies: 48Points: 113Rank: Participant
Looks good and there doesn't appear to be any errors. The only thing I would suggest and this is just a preference of mine, is to use white space to make the code more readable. Other than that, if you want to maybe shorten a few lines that have parameters that are being passed, look into splatting. Also, good job documenting and using regions to break up the code sections!
February 11, 2019 at 1:06 am #138949
Thanks Bill! I understand the spacing preference. I found, when I really started using regions, it was easier for me to keep it tight, start off reading with everything minimized, and then just walk through one small portion at a time. As for the splatting, good call. My screen at home is a little wider so I hadn't noticed. I went back and looked after your comment and I can see some overruns there. I'll check out cleaning that up. Thanks for taking the time!
February 11, 2019 at 1:10 am #138952ParticipantTopics: 3Replies: 48Points: 113Rank: Participant
If you are using vscode, there's a module you need to get and use to help you with splatting the easy way. I just found it today and it's awesome:
Seriously take a look at this.
February 12, 2019 at 1:11 am #139077
Good deal. For what I'm working on right now, still hanging around in PS ISE. Will be converting over in the next couple of months. Bookmarking for the future. Thank you!
February 12, 2019 at 12:42 pm #139135ParticipantTopics: 0Replies: 5Points: 55Rank: Member
You may want to define the [string]$logPath variable outside the Try/Catch block, or even outside the ForEach block; this need to be declared only once.
The $OutGridView variable is defined but no used (yet?). One possible other option would be to output all results to a file (CSV or text log file) in case you run that script against a very large number of computers.
It's more a personal preference; but some messages are very verbose ("Thank you. This script will now halt.") (with a typo on line 69 by the way).
Depending who is running that script and how often, you may want to reduce this. Pointing to the built-in help should be sufficient.
February 13, 2019 at 3:29 am #139264
Oh... and good eye on the typo. Thanks again!
February 13, 2019 at 3:25 am #139263
Thank you Xavier!
I completely agree on the log file and changed it just last night. Sloppy. Not sure what my original thinking was that made that make sense. I've adjusted a little, bringing it both up front and optional to change (parameter with default but can enter something else).
Regarding OutGridView, hadn't built in... was one of the things I meant by it not yet being fully written out yet. Do have that there now.
And regarding the deep commenting and verbose options, it's on purpose for the audience intended. Agreed that in an environment of folks generally friendly/comfortable with PowerShell, pretty fluffy, but meant to cater to a less familiar group and let them know more of what's going on both in code and/or progress.
Will post updated soon. Reviewing a few more things first.
February 17, 2019 at 11:59 pm #140092
I have updated things with the comments provided in mind.
- I cut my PS window to run on half my screen and shortened the lines accordingly.
- I finished out the portions that were yet to be completed.
- I moved around the csv and log path variables (now outside of the loop).
- And I redid some of the walk-through help using more helpful descriptive regions, in most cases, rather than more commenting.
Thanks for the feedback. Please do feel free to let me know if you see anything else to improve on!
The topic ‘Get-LowDriveSpace Function’ is closed to new replies.