$crop->new_name = $this->RandomStringGenerator() . "." . $crop->ext;
$this->new_name2 = $crop->new_name;
In the code above, the value of new_name2 is set by running the RandomStringGenerator() function by two times. How can I fix it?
Why do you want to do this? What is the real problem you are trying to solve with this?
Well, if running the function is the problem, why not just save the result of a function into a variable and then pass it to the variables? Unless there’s something I’m missing?
1 Like
These two lines are odd code for sure !
First, if you equal a function, it runs the function. Therefore you do not need it a second time.
In your code, you create two object values, $crop->new_name and ->new_name2. The second one depends on where $this points to. Since $this points to an object in a method, I wonder why you don’t just use one line of code to create the new_name2. Why are you making a second copy at all?
Perhaps you should explain what both lines are supposed to do! ?!?
Perhaps something like this is what you are looking for:
$myNewName = $crop->new_name = $this->RandomStringGenerator() . "." . $crop->ext;
$this->new_name2 = $myNewName;
I explain my problem in more detail:
I’m going to create an image gallery website. I need to resize and crop the uploaded image to create a thumbnail for each image. I used these codes:
cropimage.php:
<?php
class Crop
{
public $img_name;
public $tmp_img_name;
public $folder;
public $ext;
public $new_name;
function CropImage($file, $max_resolution)
{
if (file_exists($file)) {
$original_image = imagecreatefromjpeg($file);
$original_width = imagesx($original_image);
$original_height = imagesy($original_image);
//Try max-width first
if ($original_height > $original_width) {
$ratio = $max_resolution / $original_width;
$new_width = $max_resolution;
$new_height = $original_height * $ratio;
} else {
$ratio = $max_resolution / $original_height;
$new_height = $max_resolution;
$new_width = $original_width * $ratio;
}
if ($original_image) {
$new_image = imagecreatetruecolor($new_width, $new_height);
imagecopyresampled($new_image, $original_image, 0, 0, 0, 0, $new_width, $new_height, $original_width, $original_height);
$new_crop_image = imagecreatetruecolor($max_resolution, $max_resolution);
imagecopyresampled($new_crop_image, $new_image, 0, 0, 0, 0, $max_resolution, $max_resolution, $max_resolution, $max_resolution);
imagejpeg($new_crop_image, $file, 90);
}
}
}
public function RunCrop()
{
$thumb_name_name = $this->img_name;
$thumb_tmp_name = $this->tmp_img_name;
$thumb_new_name = $this->new_name;
$thumb_folder = "../public/assets/uploadThumb/";
$file = $thumb_folder . $thumb_new_name;
//Copy the original image to thumb folder
copy($this->folder . $this->new_name, $file);
//Resize file
$this->CropImage($file, "300");
}
}
saveimage.php:
<?php
include "../app/core/config.php";
include "../app/core/cropImage.php";
class UploadImage
{
private $ext = "";
private $new_name = "";
private $save_result = 0;
public function UploadIMG()
{
if (isset($_POST['img_submit'])) {
try {
//For uploading original image
$crop = new Crop();
$crop->img_name = $_FILES['image']['name'];
$crop->tmp_img_name = $_FILES['image']['tmp_name'];
$crop->folder = "../public/assets/img/";
//For finding file extension
$crop->ext = pathinfo($crop->img_name, PATHINFO_EXTENSION);
//Renaming the uploaded file
$crop->new_name = $this->RandomStringGenerator() . "." . $crop->ext;
$this->new_name = $crop->new_name;
//Moving to the desired path
move_uploaded_file($crop->tmp_img_name, $crop->folder . $crop->new_name);
$this->RegisterIntoDatabase();
//For cropping image and creating thumbnail
$crop->RunCrop();
unset($_POST);
$this->save_result = 1;
return $this->save_result;
} catch (\Throwable $th) {
$this->save_result = 2;
return $this->save_result;
}
}
}
public function RandomStringGenerator()
{
$permitted_chars = "0123456789abcdefghijkl";
$random_string = substr(str_shuffle($permitted_chars), 0, 10);
return $random_string;
}
public function RegisterIntoDatabase()
{
require "../app/core/database.php";
$bold_title = $_POST['boldtitle'];
$title = $_POST['title'];
$subtitle = $_POST['subtitle'];
$image = $this->new_name;
$sql = "INSERT INTO images (title_1, title_2, subtitle, image, thumb)
VALUES ('$bold_title', '$title', '$subtitle', '$image', '$image')";
$connection->query($sql);
}
}
The problem is that RandomStringGenerator() function is run more than once and generates two different image file name and cause the RunCrop() function to fail, because it cannot find that specific file name in the path. How can I fix it?
I added more detail to my question. Please study my code in reply section.
Why would you want to do that? Just save the file with the same name, but, add _thumb to the name.
So, Ernie.jpg would become Ernie_thumb.jpg. Saves a ton of code and problems with random characters.
( No way to debug code if you don’t know what the name of the thumb is ! )
Of course, you could also save them in a thumb folder, but, it really depends what you are attempting to do. If you are creating a gallery, you would want to keep records in your database on who posted the picture, date, time, description, etc. In that table, there would be the name of the image and the thumb name also. Then, you can use simple queries to display images based on date/time or the uploader’s ID.
Not sure if this helps or not. Just do not understand why you would want random anything in a gallery!
1 Like
The reason why I’m using random character is to avoid duplicates. For example, someone has uploaded cat.jpg from his/her computer. Someone else also has uploaded an image with the same name. This would overwrite the previous one.
I’m trying to use date and time as a part of image name (instead of random chars).
Well, if you must keep the names accurate, just use the user’s ID at the beginning of the name. And, remove it when displaying it. What type of gallery are you creating?
Using data and time is fine. But, again, two users might upload an image at the same time. You could just check for the filename already being there and if so, just add a number at the end of it. Tell us what type of gallery you want to create.
1 Like
It is an image gallery. At the first stage, I’ll have all image types in the same page but I will categorize them later. Users can sign up and after admin acceptance they will be able to upload their own image. I will add LIKE and COMMENT sections later.
Well, normally, you would make two parts to this system. One would be the actual images and the other would be the database that tracks them all. That way, you can use the database to run queries to select various images based on comment, categories, owner, etc.
And, for images, you can either keep them inside folders for each user. Such as all images from Ernie would just be kept in a folder based on Ernie’s user ID. If a user uploads two cat.jpg’s and they do not match, you could just add a number to the end of them. But, every programmer prefers their own way of storage. Adding date and time might work if the images are saved under the user’s id. Since they would not send two cat.jpg with the exact same upload date/time stamps. But, I foresee it is possible to have two separate users uploading two cat images at the same time. Very unlikely, but, quite possible.
1 Like
Firstly, about random number generation. Computers don’t do random things very well (they are pseudo-random, producing the same sequence given the same starting point), you will get duplicates, and as more of the values get used, the occurrence of duplicates will increase.
If you try to generate a random value in your code, you MUST detect duplicates by trying to INSERT the data and detecting if a duplicate index error occurs (the database column holding the value needs to be defined as a unique index.) You would loop a small number of times (to avoid a forever-loop), then use the first successfully inserted value.
Similarly, just using the date/time is not unique, and you must detect duplicates as above.
To avoid all of that, you should simply use an auto-increment id value from a database table as the filename. If a user can upload multiple images, ever, you would not use the user’s id, you would use the auto-increment id from the images table as the filename (your images table needs a user_id column to associate each row to the user it belongs to.) You would insert the row of data into the images table, then get the last insert id from that query to use as the filename.
Back to the initial problem. It appears that where the uploaded file is being moved to and where it is being copied from, for generating the thumb, are the same and there’s no code in between that is changing the value. I actually suspect that the code is being requested twice, once with $_FILES data and the second time without, but because the code doesn’t have any error checking in it, it is attempting to operate on non-existent data. What exact php errors are you getting, how exactly is the code being requested by the browser, and is there a chance that the code that’s creating and calling the UploadImage/UploadIMG() class/method is doing it twice?
Lastly, about this code. It is filled with unnecessary and unused properties and has assigned the wrong responsibility to some of the classes. OOP is not about adding class definitions around parts of your main code and adding $some_var->/$this-> in front of everything to make it work. I recommend going through the code and remove all the properties that are not needed and not used, stop copying variables to other variables for nothing, just use the original variables, removing the properties from the crop class that don’t have anything to do with cropping an image file it is receiving as a call-time parameter, and removing the database connection from the RegisterIntoDatabase() method, your main code should be responsible for making the database connection, then use dependency injection to supply that to any class that needs it.
1 Like
After all, I found what was wrong. I enabled GD in XAMPP setting and the problem solved.