What do you think about implementing command line functionality to Jarvis class? about jarvis

[AnderRasoVazquez]

In Jarvis class we will have only the user_input, speak find_action methods?

Yes, that's right. I'll start implementing it.

[sukeesh]

This looks cool!

[StefanosChaliasos]

It looks very interesting. Do you have any idea on how we could refactor the Jarvis class because it's very big?

[AnderRasoVazquez]

Yes, at the moment it looks huge, but I have some ideas. We could start doing the easiest task, one line docstrings wherever it's possible:

def help_clock(self):
        """
        Prints help about clock command.
        """
        print("Gives information about time.")

def help_clock(self):
        """Prints help about clock command."""
        print("Gives information about time.")

This will save us two lines for every command, because those docstrings are very short.

Now, let's take a look at the biggest function, weather:

def do_weather(self, s):
        """
        Get information about today's weather.
        """

        location = MEMORY.get_data('city') #Will return None if no value
        if location is None:
            loc = str(location)
            city = mapps.getLocation()['city']
            print(Fore.RED + "It appears you are in " + city + " Is this correct? (y/n)" + Fore.RESET)

            try:
                i = raw_input()
            except:
                i = input()
            if i == 'n' or i == 'no':
                print("Enter Name of city: ")
                try:
                    i = raw_input()
                except:
                    i = input()
                city = i

            mapps.weather(str(city))

            MEMORY.update_data('city', city)
            MEMORY.save()
        else:
            loc = str(location)
            city = mapps.getLocation()['city']
            if city != loc:
                print(Fore.RED + "It appears you are in " + city + ". But you set your location to " + loc + Fore.RESET)
                print(Fore.RED + "Do you want weather for " + city + " instead? (y/n)" + Fore.RESET)
                try:
                    i = raw_input()
                except:
                    i = input()
                if i == 'y' or i == 'yes':
                    try:
                        print(Fore.RED + "Would you like to set " + city + " as your new location? (y/n)" + Fore.RESET)
                        try:
                            i = raw_input()
                        except:
                            i = input()
                        if i == 'y' or i == 'yes':
                            MEMORY.update_data('city', city)
                            MEMORY.save()

                        mapps.weather(city)
                    except:
                        print(Fore.RED + "I couldn't locate you" + Fore.RESET)
                else:
                    try:
                        mapps.weather(loc)
                    except:
                        print(Fore.RED + "I couldn't locate you" + For.RESET)
            else:
                try:
                    mapps.weather(loc)
                except:
                    print(Fore.RED + "I couldn't locate you" + Fore.RESET)

Wow, 60 lines of code. This doesn't look very clean, Jarvis shouldn't be the one to handle the weather. Do you think that it might be a good idea to make a package for every skill Jarvis has? We could end up with an implementation like this:

.
└── packages
    └── weather
        └── weather.py

from packages.weather import weather
# the command might look like this
def do_weather(self, s):
        """Get information about today's weather."""
        weather.main()  # if the skill needs input -> skill.main(s)

3 lines instead of 60. Another good thing about having the implementation of the skill separated in a package is that it allows doing isolated tests from Jarvis. We can test new skills and when we are sure that they work we can add them as new commands to Jarvis. Do you have any more ideas?

[StefanosChaliasos]

Obviously we should create packages for each skill. I agree with your idea about comments too but i think we need something more efficient like having all actions in other file or files. I don't have any idea right now on how to do that.

[AnderRasoVazquez]

We can make Jarvis.py very small if we take out the command interpreter functionality to a new class.

This way we would have all the actions in other file. What do you think?

[StefanosChaliasos]

In Jarvis class we will have only the user_input, speak find_action methods? If you can implement it do it. I made some refactoring to Jarvis class (like your proposal for weather and inline comments).

[AnderRasoVazquez]

Oh, and thanks @StefanosChaliasos for considering my proposal for weather and inline comments.

[AnderRasoVazquez]

I've finished the command line implementation. I changed find_action method code that it can recognize commands if it finds those command words in the input, let's take a look at the result:

Pull request