Register
It is currently Sun Apr 22, 2018 9:39 pm

How to make this code more pretty?


All times are UTC - 6 hours


Post new topic Reply to topic  [ 3 posts ] 
Author Message
 PostPosted: Tue Jun 06, 2017 11:17 am   

Joined: Tue Jun 06, 2017 10:58 am
Posts: 2
How to make this code more pretty?

https://hastebin.com/useqazovab.bash

It works but the code is ugly.


Top
 Profile  
 PostPosted: Tue Jun 06, 2017 12:16 pm   
Moderator
User avatar

Joined: Wed May 03, 2006 2:05 pm
Posts: 273
I know what you mean! I feel the same way sometimes too. I don't see much in this script that I would format differently though... Other than some consistency things, like:

Code:
if [ foo ]; then
    bar
fi


as opposed to:
Code:
if [ foo ]
    then
        bar
fi


I like to comment my scripts a lot, and break them up into sections. It makes it easier for somebody else to understand what's happening, and helps me keep it organized in my head. Something like this... It seems redundant with some of the text that you're echoing, but to me it's easier to find things that way.

Code:
# Apply any available updates
echo "Update the system"
yum update -y

# Install some pre-requisites
echo "Install some packages"
yum -y install net-tools yum-utils epel-release wget nano vim git ntp ntpdate htop yum-cron firewalld mc sed sudo

# Set up the firewall
echo "Set up firewall"
systemctl start firewalld
firewall-cmd --permanent --zone=public --add-port=5022/tcp


There are also some things you can do, like put repeated code into functions. Here's one I use for those sshd config changes you're doing with sed, but I'm not sure if you'd consider this prettier or uglier...:

Code:
# Function to make chages to sshd config
check_sshd_config() {

    # sshd config file
    conf_file="/etc/ssh/sshd_config"

    # The setting we want to check
    key=$1

    # The value we want
    my_value=$2

    # The current value
    cur_value="$(awk '/^'${key}'/{print $2}' ${conf_file})"

    if [ "$(grep ${key} ${conf_file})" ] && [ "${cur_value}" != "${my_value}" ]; then
        echo "check_sshd_config - Updating ${key} to ${my_value}"
        sed -i -r "s%^#?(${key}).*%\1 ${my_value}%" ${conf_file}
        ssh_changes=$((ssh_changes+1))

    elif [ -z "${cur_value}" ]; then
        echo "check_sshd_config - Adding ${key} to ${conf_file} with value ${my_value}"
        echo >> ${conf_file}
        echo "${key} ${my_value}" >> ${conf_file}
        ssh_changes=$((ssh_changes+1))

    else
        echo "check_sshd_config - ${key} already matches ${my_value}"

    fi
}

# Then you can replace this...
sed -i "s/#Port 22/Port 5022/" /etc/ssh/sshd_config

# with something like this...
check_sshd_config "Port" "5022"


I wrote this, and some similar functions for a security compliance script.

I hope this helps. Honestly, I don't see much that I would change in this script other than making tab stops and if statement styles consistent.

Thanks!


Top
 Profile YIM  
 PostPosted: Wed Jun 07, 2017 11:02 am   

Joined: Tue Jun 06, 2017 10:58 am
Posts: 2
Thank you very much!


Top
 Profile  
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 3 posts ] 

All times are UTC - 6 hours


Who is online

Users browsing this forum: No registered users and 22 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
cron


BashScripts | Promote Your Page Too
Powered by phpBB © 2011 phpBB Group
© 2003 - 2011 USA LINUX USERS GROUP