[noize]

1. Replace all goto :end's with exit /b and get rid of :end

Well, it's just the same, isn't it? I had really thought about replacing them all with exit /b but I thought it was really no point. I'll do it though if there's some reason why I should better use it, let me know.

2. This isn't needed really:

Code:

cd %~dp0

Actually, it is. If I run macspoofer as an admin (which thing is needed for the actual spoofing/restoring), System32 will be the current directory, am I wrong? This way I check for the text files with default settings in the file's directory instead.

3. You set arg to nothing, and then never use it after this:

Code:

set arg=

I had removed the use of arg but forgot to remove that. I'll just edit the code.

4. There's a better way than this:

Code:

set arg1=%1
if /i "%arg1%"=="-help" set arg1=
if defined arg1 goto arg1

Try this for instance:

Code:

@echo off
if [%1] neq [] if "%1" neq "-help" goto :program
:helpdoc
echo Help requested...
pause & exit /b

:program
echo Program started...
pause

Same thing applies to other arguments that you check.

I did it just because, when dealing with parameters, I found that sometimes this code was doing it:

Code:

if [%1] neq [] if "%1" neq "-help" goto :program

While some other time it was this:

Code:

if [%1] neq [ ] if "%1" neq "-help" goto :program

I chose to then start setting the parameter to a variable and then checking if defined, as

Code:

set var=

or

Code:

set var={space}

both not define var.

5. Avoid naming block sections the same names as your variables:

Code:

:arg1

That can lead to trouble for debugging if you are writing a big script.

How? I mean, maybe there's really something I didn't consider, I'd like to know about it. But if you just meant that I could get confused with it, well, it's no worry.

ASSOC is a command as well, so you should be a bit more unique than this:

Code:

:assoc

Yeah, I know that, but it's nowhere an issue.

6. Last thing I would say is to validate the invoked script with the arguments by doing an arg count as a minimum, before you decide to use arg %1 for something %2, %3, and when you need %4 it doesn't exist...

Well, but there may be even no args or 1, 2, 3, 4, 5 or 6 valid arguments. If you meant that I could check first for the number of args and then not check for those args (like in the code two quotes beneath), well, I think it's kinda pointless.

edit: As for what @"Deque" pointed out:

Code:

if exist "con.txt" for /f "delims= tokens=*" %%k in ('type con.txt') do set con=%%k

You actually don't need type, /f is specific for files too, you just need this:

Code:

if exist "con.txt" for /f "tokens=*" %%k in (con.txt) do set con=%%k

Yeah, I know all uses of FOR, but I usually substitute even FOR 'string' with FOR /F 'echo string'. That's the same thing after all, just I find myself more comfortable.

As for those if statements:

Code:

if /i "%2"=="-c" set con=%3
if /i "%3"=="-c" set con=%4
if /i "%4"=="-c" set con=%5
if /i "%5"=="-c" set con=%6
if /i "%6"=="-c" set con=%7

You might be able to check then just shift the arguments. And assign to the same one since shifting is downwards. You aren't dealing with more than 9 arguments though, otherwise I usually create my own pseudo array of arguments...

You're right. I had a bit of troubles in making all of that block of code shorter using different commands and I then chose to leave it like that. Though, I didn't think of using the SHIFT command for this kind of things. Actually, if I would shift it like that here it would take more code than what that block of code is to take the args back in order (I'm not saying it's too hard, just it's more code than that). Yeah, by shifting all commands until I find for all times this issue comes to me, there wouldn't be the issue.

I'll improve my code later. Thanks.