Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like ftp_connection often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use ftp_connection, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
730 | class ftp_connection |
||
731 | { |
||
732 | /** |
||
733 | * @var string Holds the connection response |
||
734 | */ |
||
735 | public $connection; |
||
736 | |||
737 | /** |
||
738 | * @var string Holds any errors |
||
739 | */ |
||
740 | public $error; |
||
741 | |||
742 | /** |
||
743 | * @var string Holds the last message from the server |
||
744 | */ |
||
745 | public $last_message; |
||
746 | |||
747 | /** |
||
748 | * @var boolean Whether or not this is a passive connection |
||
749 | */ |
||
750 | public $pasv; |
||
751 | |||
752 | /** |
||
753 | * Create a new FTP connection... |
||
754 | * |
||
755 | * @param string $ftp_server The server to connect to |
||
756 | * @param int $ftp_port The port to connect to |
||
757 | * @param string $ftp_user The username |
||
758 | * @param string $ftp_pass The password |
||
759 | */ |
||
760 | public function __construct($ftp_server, $ftp_port = 21, $ftp_user = 'anonymous', $ftp_pass = '[email protected]') |
||
761 | { |
||
762 | // Initialize variables. |
||
763 | $this->connection = 'no_connection'; |
||
764 | $this->error = false; |
||
765 | $this->pasv = array(); |
||
766 | |||
767 | if ($ftp_server !== null) |
||
768 | $this->connect($ftp_server, $ftp_port, $ftp_user, $ftp_pass); |
||
769 | } |
||
770 | |||
771 | /** |
||
772 | * Connects to a server |
||
773 | * |
||
774 | * @param string $ftp_server The address of the server |
||
775 | * @param int $ftp_port The port |
||
776 | * @param string $ftp_user The username |
||
777 | * @param string $ftp_pass The password |
||
778 | */ |
||
779 | public function connect($ftp_server, $ftp_port = 21, $ftp_user = 'anonymous', $ftp_pass = '[email protected]') |
||
780 | { |
||
781 | if (strpos($ftp_server, 'ftp://') === 0) |
||
782 | $ftp_server = substr($ftp_server, 6); |
||
783 | elseif (strpos($ftp_server, 'ftps://') === 0) |
||
784 | $ftp_server = 'ssl://' . substr($ftp_server, 7); |
||
785 | if (strpos($ftp_server, 'http://') === 0) |
||
786 | $ftp_server = substr($ftp_server, 7); |
||
787 | elseif (strpos($ftp_server, 'https://') === 0) |
||
788 | $ftp_server = substr($ftp_server, 8); |
||
789 | $ftp_server = strtr($ftp_server, array('/' => '', ':' => '', '@' => '')); |
||
790 | |||
791 | // Connect to the FTP server. |
||
792 | $this->connection = @fsockopen($ftp_server, $ftp_port, $err, $err, 5); |
||
793 | if (!$this->connection) |
||
794 | { |
||
795 | $this->error = 'bad_server'; |
||
796 | $this->last_message = 'Invalid Server'; |
||
797 | return; |
||
798 | } |
||
799 | |||
800 | // Get the welcome message... |
||
801 | if (!$this->check_response(220)) |
||
802 | { |
||
803 | $this->error = 'bad_response'; |
||
804 | $this->last_message = 'Bad Response'; |
||
805 | return; |
||
806 | } |
||
807 | |||
808 | // Send the username, it should ask for a password. |
||
809 | fwrite($this->connection, 'USER ' . $ftp_user . "\r\n"); |
||
810 | |||
811 | if (!$this->check_response(331)) |
||
812 | { |
||
813 | $this->error = 'bad_username'; |
||
814 | $this->last_message = 'Invalid Username'; |
||
815 | return; |
||
816 | } |
||
817 | |||
818 | // Now send the password... and hope it goes okay. |
||
819 | |||
820 | fwrite($this->connection, 'PASS ' . $ftp_pass . "\r\n"); |
||
821 | if (!$this->check_response(230)) |
||
822 | { |
||
823 | $this->error = 'bad_password'; |
||
824 | $this->last_message = 'Invalid Password'; |
||
825 | return; |
||
826 | } |
||
827 | } |
||
828 | |||
829 | /** |
||
830 | * Changes to a directory (chdir) via the ftp connection |
||
831 | * |
||
832 | * @param string $ftp_path The path to the directory we want to change to |
||
833 | * @return boolean Whether or not the operation was successful |
||
834 | */ |
||
835 | public function chdir($ftp_path) |
||
836 | { |
||
837 | if (!is_resource($this->connection)) |
||
838 | return false; |
||
839 | |||
840 | // No slash on the end, please... |
||
841 | if ($ftp_path !== '/' && substr($ftp_path, -1) === '/') |
||
842 | $ftp_path = substr($ftp_path, 0, -1); |
||
843 | |||
844 | fwrite($this->connection, 'CWD ' . $ftp_path . "\r\n"); |
||
845 | if (!$this->check_response(250)) |
||
846 | { |
||
847 | $this->error = 'bad_path'; |
||
848 | return false; |
||
849 | } |
||
850 | |||
851 | return true; |
||
852 | } |
||
853 | |||
854 | /** |
||
855 | * Changes a files atrributes (chmod) |
||
856 | * |
||
857 | * @param string $ftp_file The file to CHMOD |
||
858 | * @param int|string $chmod The value for the CHMOD operation |
||
859 | * @return boolean Whether or not the operation was successful |
||
860 | */ |
||
861 | public function chmod($ftp_file, $chmod) |
||
862 | { |
||
863 | if (!is_resource($this->connection)) |
||
864 | return false; |
||
865 | |||
866 | if ($ftp_file == '') |
||
867 | $ftp_file = '.'; |
||
868 | |||
869 | // Do we have a file or a dir? |
||
870 | $is_dir = is_dir($ftp_file); |
||
871 | $is_writable = false; |
||
872 | |||
873 | // Set different modes. |
||
874 | $chmod_values = $is_dir ? array(0750, 0755, 0775, 0777) : array(0644, 0664, 0666); |
||
875 | |||
876 | foreach ($chmod_values as $val) |
||
877 | { |
||
878 | // If it's writable, break out of the loop. |
||
879 | if (is_writable($ftp_file)) |
||
880 | { |
||
881 | $is_writable = true; |
||
882 | break; |
||
883 | } |
||
884 | |||
885 | else |
||
886 | { |
||
887 | // Convert the chmod value from octal (0777) to text ("777"). |
||
888 | fwrite($this->connection, 'SITE CHMOD ' . decoct($val) . ' ' . $ftp_file . "\r\n"); |
||
889 | if (!$this->check_response(200)) |
||
890 | { |
||
891 | $this->error = 'bad_file'; |
||
892 | break; |
||
893 | } |
||
894 | } |
||
895 | } |
||
896 | return $is_writable; |
||
897 | } |
||
898 | |||
899 | /** |
||
900 | * Deletes a file |
||
901 | * |
||
902 | * @param string $ftp_file The file to delete |
||
903 | * @return boolean Whether or not the operation was successful |
||
904 | */ |
||
905 | public function unlink($ftp_file) |
||
906 | { |
||
907 | // We are actually connected, right? |
||
908 | if (!is_resource($this->connection)) |
||
909 | return false; |
||
910 | |||
911 | // Delete file X. |
||
912 | fwrite($this->connection, 'DELE ' . $ftp_file . "\r\n"); |
||
913 | if (!$this->check_response(250)) |
||
914 | { |
||
915 | fwrite($this->connection, 'RMD ' . $ftp_file . "\r\n"); |
||
916 | |||
917 | // Still no love? |
||
918 | if (!$this->check_response(250)) |
||
919 | { |
||
920 | $this->error = 'bad_file'; |
||
921 | return false; |
||
922 | } |
||
923 | } |
||
924 | |||
925 | return true; |
||
926 | } |
||
927 | |||
928 | /** |
||
929 | * Reads the response to the command from the server |
||
930 | * |
||
931 | * @param string $desired The desired response |
||
932 | * @return boolean Whether or not we got the desired response |
||
933 | */ |
||
934 | public function check_response($desired) |
||
944 | } |
||
945 | |||
946 | /** |
||
947 | * Used to create a passive connection |
||
948 | * |
||
949 | * @return boolean Whether the passive connection was created successfully |
||
950 | */ |
||
951 | public function passive() |
||
952 | { |
||
953 | // We can't create a passive data connection without a primary one first being there. |
||
954 | if (!is_resource($this->connection)) |
||
955 | return false; |
||
956 | |||
957 | // Request a passive connection - this means, we'll talk to you, you don't talk to us. |
||
958 | @fwrite($this->connection, 'PASV' . "\r\n"); |
||
959 | $time = time(); |
||
960 | do |
||
961 | $response = fgets($this->connection, 1024); |
||
962 | while (strpos($response, ' ', 3) !== 3 && time() - $time < 5); |
||
963 | |||
964 | // If it's not 227, we weren't given an IP and port, which means it failed. |
||
965 | if (strpos($response, '227 ') !== 0) |
||
966 | { |
||
967 | $this->error = 'bad_response'; |
||
968 | return false; |
||
969 | } |
||
970 | |||
971 | // Snatch the IP and port information, or die horribly trying... |
||
972 | if (preg_match('~\((\d+),\s*(\d+),\s*(\d+),\s*(\d+),\s*(\d+)(?:,\s*(\d+))\)~', $response, $match) == 0) |
||
973 | { |
||
974 | $this->error = 'bad_response'; |
||
975 | return false; |
||
976 | } |
||
977 | |||
978 | // This is pretty simple - store it for later use ;). |
||
979 | $this->pasv = array('ip' => $match[1] . '.' . $match[2] . '.' . $match[3] . '.' . $match[4], 'port' => $match[5] * 256 + $match[6]); |
||
980 | |||
981 | return true; |
||
982 | } |
||
983 | |||
984 | /** |
||
985 | * Creates a new file on the server |
||
986 | * |
||
987 | * @param string $ftp_file The file to create |
||
988 | * @return boolean Whether or not the file was created successfully |
||
989 | */ |
||
990 | public function create_file($ftp_file) |
||
1021 | } |
||
1022 | |||
1023 | /** |
||
1024 | * Generates a directory listing for the current directory |
||
1025 | * |
||
1026 | * @param string $ftp_path The path to the directory |
||
1027 | * @param bool $search Whether or not to get a recursive directory listing |
||
1028 | * @return string|boolean The results of the command or false if unsuccessful |
||
1029 | */ |
||
1030 | public function list_dir($ftp_path = '', $search = false) |
||
1031 | { |
||
1032 | // Are we even connected...? |
||
1033 | if (!is_resource($this->connection)) |
||
1034 | return false; |
||
1035 | |||
1036 | // Passive... non-agressive... |
||
1037 | if (!$this->passive()) |
||
1038 | return false; |
||
1039 | |||
1040 | // Get the listing! |
||
1041 | fwrite($this->connection, 'LIST -1' . ($search ? 'R' : '') . ($ftp_path == '' ? '' : ' ' . $ftp_path) . "\r\n"); |
||
1042 | |||
1043 | // Connect, assuming we've got a connection. |
||
1044 | $fp = @fsockopen($this->pasv['ip'], $this->pasv['port'], $err, $err, 5); |
||
1045 | if (!$fp || !$this->check_response(array(150, 125))) |
||
1046 | { |
||
1047 | $this->error = 'bad_response'; |
||
1048 | @fclose($fp); |
||
1049 | return false; |
||
1050 | } |
||
1051 | |||
1052 | // Read in the file listing. |
||
1053 | $data = ''; |
||
1054 | while (!feof($fp)) |
||
1055 | $data .= fread($fp, 4096); |
||
1056 | fclose($fp); |
||
1057 | |||
1058 | // Everything go okay? |
||
1059 | if (!$this->check_response(226)) |
||
1060 | { |
||
1061 | $this->error = 'bad_response'; |
||
1062 | return false; |
||
1063 | } |
||
1064 | |||
1065 | return $data; |
||
1066 | } |
||
1067 | |||
1068 | /** |
||
1069 | * Determines the current directory we are in |
||
1070 | * |
||
1071 | * @param string $file The name of a file |
||
1072 | * @param string $listing A directory listing or null to generate one |
||
1073 | * @return string|boolean The name of the file or false if it wasn't found |
||
1074 | */ |
||
1075 | public function locate($file, $listing = null) |
||
1076 | { |
||
1077 | if ($listing === null) |
||
1078 | $listing = $this->list_dir('', true); |
||
1079 | $listing = explode("\n", $listing); |
||
1080 | |||
1081 | @fwrite($this->connection, 'PWD' . "\r\n"); |
||
1082 | $time = time(); |
||
1083 | do |
||
1084 | $response = fgets($this->connection, 1024); |
||
1085 | while ($response[3] != ' ' && time() - $time < 5); |
||
1086 | |||
1087 | // Check for 257! |
||
1088 | if (preg_match('~^257 "(.+?)" ~', $response, $match) != 0) |
||
1089 | $current_dir = strtr($match[1], array('""' => '"')); |
||
1090 | else |
||
1091 | $current_dir = ''; |
||
1092 | |||
1093 | for ($i = 0, $n = count($listing); $i < $n; $i++) |
||
1094 | { |
||
1095 | if (trim($listing[$i]) == '' && isset($listing[$i + 1])) |
||
1096 | { |
||
1097 | $current_dir = substr(trim($listing[++$i]), 0, -1); |
||
1098 | $i++; |
||
1099 | } |
||
1100 | |||
1101 | // Okay, this file's name is: |
||
1102 | $listing[$i] = $current_dir . '/' . trim(strlen($listing[$i]) > 30 ? strrchr($listing[$i], ' ') : $listing[$i]); |
||
1103 | |||
1104 | if ($file[0] == '*' && substr($listing[$i], -(strlen($file) - 1)) == substr($file, 1)) |
||
1105 | return $listing[$i]; |
||
1106 | if (substr($file, -1) == '*' && substr($listing[$i], 0, strlen($file) - 1) == substr($file, 0, -1)) |
||
1107 | return $listing[$i]; |
||
1108 | if (basename($listing[$i]) == $file || $listing[$i] == $file) |
||
1109 | return $listing[$i]; |
||
1110 | } |
||
1111 | |||
1112 | return false; |
||
1113 | } |
||
1114 | |||
1115 | /** |
||
1116 | * Creates a new directory on the server |
||
1117 | * |
||
1118 | * @param string $ftp_dir The name of the directory to create |
||
1119 | * @return boolean Whether or not the operation was successful |
||
1120 | */ |
||
1121 | public function create_dir($ftp_dir) |
||
1122 | { |
||
1123 | // We must be connected to the server to do something. |
||
1124 | if (!is_resource($this->connection)) |
||
1125 | return false; |
||
1126 | |||
1127 | // Make this new beautiful directory! |
||
1128 | fwrite($this->connection, 'MKD ' . $ftp_dir . "\r\n"); |
||
1129 | if (!$this->check_response(257)) |
||
1130 | { |
||
1131 | $this->error = 'bad_file'; |
||
1132 | return false; |
||
1133 | } |
||
1134 | |||
1135 | return true; |
||
1136 | } |
||
1137 | |||
1138 | /** |
||
1139 | * Detects the current path |
||
1140 | * |
||
1141 | * @param string $filesystem_path The full path from the filesystem |
||
1142 | * @param string $lookup_file The name of a file in the specified path |
||
1143 | * @return array An array of detected info - username, path from FTP root and whether or not the current path was found |
||
1144 | */ |
||
1145 | public function detect_path($filesystem_path, $lookup_file = null) |
||
1188 | } |
||
1189 | |||
1190 | /** |
||
1191 | * Close the ftp connection |
||
1192 | * |
||
1193 | * @return boolean Always returns true |
||
1194 | */ |
||
1195 | public function close() |
||
1202 | } |
||
1203 | } |
||
1204 | |||
1205 | ?> |
Our type inference engine has found an assignment to a property that is incompatible with the declared type of that property.
Either this assignment is in error or the assigned type should be added to the documentation/type hint for that property..