If you decide to use 20MHz then obviously change the DEFINE OSC statement. Whether you decide to go 20MHz depends if you can satisfy your application requirements whilst running at 4MHz. 4MHz parts are usually cheaper in production.

If you used a 24LC512, then it could have shared the I2C bus with the DS1307 and save yourself a heap of PIC pins in the process. As it is you've forgotten about the EEPROM's SO pin unless of course the EEPROM is going to be used as a WRITE ONLY device (ie datalogger application and you're going to crow-bar* it off the circuit-board to read-back)...

*Crow-Bar is a Bristish term for a chip-extractor/removal tool aka 'Wrecker-Bar' in the USA.