0

I have been working on my discord bot recently and I want to create a command which sends a random photo from given array. At present I have this code:

[Command("sendpic"), Summary("")]
    public async Task SendPic()
    {
        string[] RandomPic = { "https://images.unsplash.com/photo-1536746803623-cef87080bfc8?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjEyMDd9&auto=format&fit=crop&w=1000&q=80",
            "https://s3.amazonaws.com/artallnight/static/files/2018/08/14154730/back-1024x532.jpg",
            "https://www.abc.net.au/news/image/9776766-3x2-700x467.jpg",
            "https://static1.squarespace.com/static/58d2d1d003596ef5151dd694/t/5911277b893fc011d4e8543d/1494296445782/stars2.jpg?format=1500w"
        };
            Random random = new Random();
            int randomNumber = random.Next(0, 3);
            EmbedBuilder d = new EmbedBuilder();
            d.WithColor(120, 40, 23);

        if (randomNumber == 1)
        {
            await Context.Channel.SendMessageAsync(RandomPic[1]);
        } else if (randomNumber == 2) {
            await Context.Channel.SendMessageAsync(RandomPic[2]);
        } else if (randomNumber == 3){
            await Context.Channel.SendMessageAsync(RandomPic[3]);
        } else {
            await Context.Channel.SendMessageAsync(RandomPic[0]);
        }
    }

but it seems to be not working since it only sends 1 picture whenever I call the "sendpic" command. How do I make it to send given pics from array(each one at every call?)

yuvi
  • 1
  • 3
  • Why do you check the value of ```randomNumber```? You can simply use ```randomNumber``` in the array. ```Context.Channel.SendMessageAsync(RandomPic[randomNumber]);```. I'd also suggest initializing the random object outside the function. Chances are high that you'll always get the same picture, refer to [this](https://stackoverflow.com/questions/767999/random-number-generator-only-generating-one-random-number). Also, you're generating numbers between 0 and 2 because the MaxValue is **exclusive**. – devsmn Feb 28 '19 at 10:30
  • 1
    Btw, `random.Next(0, 3)` will never return 3. The `maxValue` is exclusive, you need to write `random.Next(0, 4)` if you expect to get 3 as the result. – Sergey Shevchenko Feb 28 '19 at 10:34
  • alright it seems that I fixed this. It now gives back other picture. Thanks for your help! – yuvi Feb 28 '19 at 10:37

1 Answers1

1

The problem is you are creating a new instance of Random everytime your command is called. This class is not meant to be used like that. You should have a static variable outside the scope of the SendPic() method, like this:

public class YourModule : ModuleBase<SocketCommandContext>
{
    private static readonly Random random = new Random();

    //Your commands here
}

Then I would just use it like this:

    [Command("sendpic"), Summary("")]
    public async Task SendPic()
    {
        string[] RandomPic = 
        { 
            "https://images.unsplash.com/photo-1536746803623-cef87080bfc8?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjEyMDd9&auto=format&fit=crop&w=1000&q=80",
            "https://s3.amazonaws.com/artallnight/static/files/2018/08/14154730/back-1024x532.jpg",
            "https://www.abc.net.au/news/image/9776766-3x2-700x467.jpg",
            "https://static1.squarespace.com/static/58d2d1d003596ef5151dd694/t/5911277b893fc011d4e8543d/1494296445782/stars2.jpg?format=1500w"
        };
            int randomNumber = random.Next(0, RandomPic.Length);
            EmbedBuilder d = new EmbedBuilder();
            d.WithColor(120, 40, 23);

            await Context.Channel.SendMessageAsync(RandomPic[randomNumber]);
    }

By the way, you don't need to check the results number by number. It doesn't escalate well (imagine checking for a number between 1 and 1000 instead). Check how I made RandomPic.Length the upper limit for randomNumber, removed the if statement and called RandomPic[randomNumber] directly (which retrieves the element at randomNumber position of the RandomPic array.