New to programming and Arduino. Would like someone with experience to grade me.

Hi guys, noob here. just trying to teach myself how to code. This is my first attempt at completely coding something from scratch.

Basically Ive got a circuit hooked up with a button, potentiometer, photoresistor, red led, white led, and a blue led. When you hold down the button the lights will blink. The blink speed is controlled by the potentiometer. you can also get the lights to blink my covering the photoresistor.

However, im not sure if its my code thats messed up or my circuit. If anyone would like to look over my code and give me some pointers or easier ways to do things. Any help or tips would be greatly appreciated. Mostly just want to make sure im not starting bad habits early on. If you see any redundant code or simpler wasy of doing something, please feel free to let me know.

Thanks.

int redLedPin=6;                 //Set Red LED
int whiteLedPin=7;              //Set White LED
int blueLedPin=10;             //Set Blue LED
int potPin=A0;                //Set Pot Pin
int photoPin=A1;
int button=7;               //Set button pin
int photoReading;
int potReading;           //raw value from pot
int mapped;              //mapped value from pot
boolean buttonState;    // name for button state(true or false)
int blinkTime=150;     // my mapped value, its easier to change it here
int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.


void setup() {
 Serial.begin(9600);                  //begin serial monitor
 pinMode(redLedPin, OUTPUT);         //set red LED pin as output
 pinMode(whiteLedPin, OUTPUT);      //set white LED pin as output
 pinMode(blueLedPin, OUTPUT);      //set blue LED pin as output
 pinMode(potPin, INPUT);          //set potPin as input
 pinMode(button, INPUT);         //set button pin as input
 pinMode(photoPin, INPUT);

}


void loop() {
  potReading=(analogRead(potPin));                          // Getting pot ready
  photoReading=(analogRead(photoPin));                     // getting photocell ready
  Serial.print("Pot reading:  ");                         // printing out value so i can see it
  Serial.print( mapped);
  mapped = map (potReading, 0, 1023, 10, blinkTime);    // mapping out value of pot for use in seconds
  buttonState= digitalRead (button);                   // getting button ready
  Serial.print("         ");
  Serial.print("Button State:   ");
  Serial.print(buttonState);                        //printing out button State in serial monitor
  Serial.print("         ");
  Serial.print("Photo State:    ");
  Serial.print("         ");
  Serial.println(photoReading);


  if (buttonState==false || photoReading <= photoSet ) {         //basically going to flash like a cop car while you hold down the button or if there is not enough light
    digitalWrite(redLedPin, HIGH);
    delay(mapped);                                             //pot controls the speed at which the leds flash
    digitalWrite(redLedPin, LOW);
    delay(mapped);
    digitalWrite(whiteLedPin, HIGH);
    delay(mapped);
    digitalWrite(whiteLedPin, LOW);
    delay(mapped);
    digitalWrite(blueLedPin, HIGH);
    delay(mapped);
    digitalWrite(blueLedPin, LOW);
    delay(mapped);
    
  }

   
    
}

Blinky_button_annoying_pot_thingy.ino (2.33 KB)

I am not clear whether your program works and you want tips to improve it or whether it does not work.

Either way, please post your code here to avoid the need to download it and use code tags when you do.

  Serial.print("Pot reading:  ");                         // printing out value so i can see it
  Serial.print( mapped);
  mapped = map (potReading, 0, 1023, 10, blinkTime);    // mapping out value of pot for use in seconds

(Note the use of code tags)

Why are you printing the value of mapped before you map it from the pot reading ?

int redLedPin=6;                 //Set Red LED
int whiteLedPin=7;              //Set White LED
int blueLedPin=10;             //Set Blue LED
int potPin=A0;                //Set Pot Pin
int photoPin=A1;
int button=7;               //Set button pin
int photoReading;
int potReading;           //raw value from pot
int mapped;              //mapped value from pot
boolean buttonState;    // name for button state(true or false)
int blinkTime=150;     // my mapped value, its easier to change it here
int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.


void setup() {
 Serial.begin(9600);                  //begin serial monitor
 pinMode(redLedPin, OUTPUT);         //set red LED pin as output
 pinMode(whiteLedPin, OUTPUT);      //set white LED pin as output
 pinMode(blueLedPin, OUTPUT);      //set blue LED pin as output
 pinMode(potPin, INPUT);          //set potPin as input
 pinMode(button, INPUT);         //set button pin as input
 pinMode(photoPin, INPUT);

}


void loop() {
  potReading=(analogRead(potPin));                          // Getting pot ready
  photoReading=(analogRead(photoPin));                     // getting photocell ready
  Serial.print("Pot reading:  ");                         // printing out value so i can see it
  Serial.print( mapped);
  mapped = map (potReading, 0, 1023, 10, blinkTime);    // mapping out value of pot for use in seconds
  buttonState= digitalRead (button);                   // getting button ready
  Serial.print("         ");
  Serial.print("Button State:   ");
  Serial.print(buttonState);                        //printing out button State in serial monitor
  Serial.print("         ");
  Serial.print("Photo State:    ");
  Serial.print("         ");
  Serial.println(photoReading);


  if (buttonState==false || photoReading <= photoSet ) {         //basically going to flash like a cop car while you hold down the button or if there is not enough light
    digitalWrite(redLedPin, HIGH);
    delay(mapped);                                             //pot controls the speed at which the leds flash
    digitalWrite(redLedPin, LOW);
    delay(mapped);
    digitalWrite(whiteLedPin, HIGH);
    delay(mapped);
    digitalWrite(whiteLedPin, LOW);
    delay(mapped);
    digitalWrite(blueLedPin, HIGH);
    delay(mapped);
    digitalWrite(blueLedPin, LOW);
    delay(mapped);
    
  }

   
    
}

Helping the guy out

A wiring diagram (schematic) would help, too. Particularly how the button switch is wired.

UKHeliBob:   Serial.print("Pot reading:  ");                        // printing out value so i can see it   Serial.print( mapped);   mapped = map (potReading, 0, 1023, 10, blinkTime);    // mapping out value of pot for use in seconds

(Note the use of code tags)

Why are you printing the value of mapped before you map it from the pot reading ?

haha thank you, i can't believe i missed that, the joys of learning.

Here’s some pictures. Best I can do for now while I’m at work lol.

  • Always do Tools > Auto Format on your code. If you're using the Arduino Web Editor then you're SOL on that. You could use an external tool such as Artistic Style, which is what the standard IDE uses.
  • Remove unnecessary blank lines from your code before posting to the forum. One or two to separate code into logical sections is fine but large spaces for no reason just make for more scrolling.
  • Quit this weird thing you're doing with the comment spacing. My advice is just to do a single tab to separate the comments from the code. Some people try to make it all pretty by lining all the comments up but it just seems like more work than it's worth to me.
  • Use a consistent style. In some of your code you have a space between the function name and the parentesis, other places no space. When there is no functional difference you are free to make your own choices but it's generally considered best practice to follow existing style conventions. Thus I would recommend following the style consistently used in official Arduino example code. Some people have different preferences and that's fine too, the important thing is to be consistent.
  • Whenever you have a variable that's value shouldn't change (such as your pin variables) use the const keyword: https://www.arduino.cc/en/Reference/Const
  • Generally you should use the F() macro for string literals as you use in your Serial prints. This causes them to be placed in the more plentiful flash memory instead of SRAM. This is not an absolute rule because there are times when it is better not to use the F() macro. You may actually have a shortage of flash and extra SRAM, you may be more concerned with speed than memory usage and in that case the code will be faster without the F() macro. In your code you probably have plenty of free memory and are not concerned with speed so it's not really a big deal either way but it can become very important with more complex projects.
  • Don't use delay. Please study File > Examples > 02.Digital > BlinkWithoutDelay and the associated tutorial page until you completely understand it. This is very important.

Hi tpayne++,

What I'm about to say is my own opinion and also my own style. You should know that in programming you should develop your own style. Take anything that I written down with a grain salt.

first of

int redLedPin=6;                 //Set Red LED
int whiteLedPin=7;              //Set White LED
int blueLedPin=10;             //Set Blue LED
int potPin=A0;                //Set Pot Pin
int photoPin=A1;
int button=7;               //Set button pin
int photoReading;
int potReading;           //raw value from pot
int mapped;              //mapped value from pot
boolean buttonState;    // name for button state(true or false)
int blinkTime=150;     // my mapped value, its easier to change it here
int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.

The formatting of the comment is annoying as per glance. I would prefer the comment to start all in parallel to each other.

int redLedPin=6;      //Set Red LED
int whiteLedPin=7;    //Set White LED
int blueLedPin=10;    //Set Blue LED
int potPin=A0;        //Set Pot Pin
int photoPin=A1;
int button=7;         //Set button pin
int photoReading;
int potReading;       //raw value from pot
int mapped;           //mapped value from pot
boolean buttonState;  // name for button state(true or false)
int blinkTime=150;    // my mapped value, its easier to change it here
int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.

I also firmly believe that if the line of code is self-explanatory, you should not add any comment.

int redLedPin=6;
int whiteLedPin=7;
int blueLedPin=10;
int potPin=A0;
int photoPin=A1;
int button=7;
int photoReading;
int potReading;
int mapped;           //mapped value from pot
boolean buttonState;  // name for button state(true or false)
int blinkTime=150;    // my mapped value, its easier to change it here
int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.

The the first 8 line are not commented cause for me it is self-explanatory.

I notice that the first 6 line is the declaration of INPUT and OUTPUT pin. For me, I would prefer to use a constant and also use the smaller variable type to reduce amount of memory used in the Arduino such as

const uint8_t redLedPin=6;
const uint8_t whiteLedPin=7;
const uint8_t blueLedPin=10;
const uint8_t button=7;
const uint8_t potPin=A0;
const uint8_t photoPin=A1;
int photoReading;
int potReading;
int mapped;           //mapped value from pot
boolean buttonState;  // name for button state(true or false)
const int blinkTime=150;    // my mapped value, its easier to change it here
const int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.

The rest I don't feel that I have to comment a lot about except for

Serial.print("Pot reading:  ");                         // printing out value so i can see it
  Serial.print( mapped);
  mapped = map (potReading, 0, 1023, 10, blinkTime);    // mapping out value of pot for use in seconds
  buttonState= digitalRead (button);                   // getting button ready
  Serial.print("         ");
  Serial.print("Button State:   ");
  Serial.print(buttonState);                        //printing out button State in serial monitor
  Serial.print("         ");
  Serial.print("Photo State:    ");
  Serial.print("         ");
  Serial.println(photoReading);

You see in Arduino theres a hidden function... you could save room if you used F function for constant string.

potReading=(analogRead(potPin));                          // Getting pot ready
  photoReading=(analogRead(photoPin));                     // getting photocell ready
  Serial.print(F("Pot reading:  "));                         // printing out value so i can see it
  Serial.print( mapped);
  mapped = map (potReading, 0, 1023, 10, blinkTime);    // mapping out value of pot for use in seconds
  buttonState= digitalRead (button);                   // getting button ready
  Serial.print(F("         "));
  Serial.print(F("Button State:   "));
  Serial.print(buttonState);                        //printing out button State in serial monitor
  Serial.print(F("         "));
  Serial.print(F("Photo State:    "));
  Serial.print(F("         "));
  Serial.println(photoReading);

I don't know if your code works, but the flow is clean and I can see what you are trying to do.

pert: - Always do Tools > Auto Format on your code. If you're using the Arduino Web Editor then you're SOL on that. You could use an external tool such as Artistic Style, which is what the standard IDE uses. - Remove unnecessary blank lines from your code before posting to the forum. One or two to separate code into logical sections is fine but large spaces for no reason just make for more scrolling. - Quit this weird thing you're doing with the comment spacing. My advice is just to do a single tab to separate the comments from the code. Some people try to make it all pretty by lining all the comments up but it just seems like more work than it's worth to me. - Use a consistent style. In some of your code you have a space between the function name and the parentesis, other places no space. When there is no functional difference you are free to make your own choices but it's generally considered best practice to follow existing style conventions. Thus I would recommend following the style consistently used in official Arduino example code. Some people have different preferences and that's fine too, the important thing is to be consistent. - Whenever you have a variable that's value shouldn't change (such as your pin variables) use the const keyword: https://www.arduino.cc/en/Reference/Const - Generally you should use the F() macro for string literals as you use in your Serial prints. This causes them to be placed in the more plentiful flash memory instead of SRAM. This is not an absolute rule because there are times when it is better not to use the F() macro. You may actually have a shortage of flash and extra SRAM, you may be more concerned with speed than memory usage and in that case the code will be faster without the F() macro. In your code you probably have plenty of free memory and are not concerned with speed so it's not really a big deal either way but it can become very important with more complex projects. - Don't use delay. Please study File > Examples > 02.Digital > BlinkWithoutDelay and the associated tutorial page until you completely understand it. This is very important.

thank you! i will check that stuff out. This was my first weekend messing with this stuff i still have tons to learn!

Ashraf_Zolkopli:
Hi tpayne++,

What I’m about to say is my own opinion and also my own style. You should know that in programming you should develop your own style. Take anything that I written down with a grain salt.

first of

int redLedPin=6;                 //Set Red LED

int whiteLedPin=7;              //Set White LED
int blueLedPin=10;             //Set Blue LED
int potPin=A0;                //Set Pot Pin
int photoPin=A1;
int button=7;               //Set button pin
int photoReading;
int potReading;           //raw value from pot
int mapped;              //mapped value from pot
boolean buttonState;    // name for button state(true or false)
int blinkTime=150;     // my mapped value, its easier to change it here
int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.




The formatting of the comment is annoying as per glance. I would prefer the comment to start all in parallel to each other.



int redLedPin=6;      //Set Red LED
int whiteLedPin=7;    //Set White LED
int blueLedPin=10;    //Set Blue LED
int potPin=A0;        //Set Pot Pin
int photoPin=A1;
int button=7;         //Set button pin
int photoReading;
int potReading;       //raw value from pot
int mapped;           //mapped value from pot
boolean buttonState;  // name for button state(true or false)
int blinkTime=150;    // my mapped value, its easier to change it here
int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.




I also firmly believe that if the line of code is self-explanatory, you should not add any comment.



int redLedPin=6;
int whiteLedPin=7;
int blueLedPin=10;
int potPin=A0;
int photoPin=A1;
int button=7;
int photoReading;
int potReading;
int mapped;           //mapped value from pot
boolean buttonState;  // name for button state(true or false)
int blinkTime=150;    // my mapped value, its easier to change it here
int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.




The the first 8 line are not commented cause for me it is self-explanatory.


I notice that the first 6 line is the declaration of INPUT and OUTPUT pin. For me, I would prefer to use a constant and also use the smaller variable type to reduce amount of memory used in the Arduino such as 



const uint8_t redLedPin=6;
const uint8_t whiteLedPin=7;
const uint8_t blueLedPin=10;
const uint8_t button=7;
const uint8_t potPin=A0;
const uint8_t photoPin=A1;
int photoReading;
int potReading;
int mapped;           //mapped value from pot
boolean buttonState;  // name for button state(true or false)
const int blinkTime=150;    // my mapped value, its easier to change it here
const int photoSet=20;      // Set where you want the Photo sensor to turn lights on here.




The rest I don't feel that I have to comment a lot about except for 


Serial.print("Pot reading:  “);                         // printing out value so i can see it
 Serial.print( mapped);
 mapped = map (potReading, 0, 1023, 10, blinkTime);    // mapping out value of pot for use in seconds
 buttonState= digitalRead (button);                   // getting button ready
 Serial.print(”         ");
 Serial.print("Button State:   “);
 Serial.print(buttonState);                        //printing out button State in serial monitor
 Serial.print(”         ");
 Serial.print("Photo State:    “);
 Serial.print(”         ");
 Serial.println(photoReading);




You see in Arduino theres a hidden function... you could save room if you used F function for constant string.



potReading=(analogRead(potPin));                          // Getting pot ready
 photoReading=(analogRead(photoPin));                     // getting photocell ready
 Serial.print(F("Pot reading:  “));                         // printing out value so i can see it
 Serial.print( mapped);
 mapped = map (potReading, 0, 1023, 10, blinkTime);    // mapping out value of pot for use in seconds
 buttonState= digitalRead (button);                   // getting button ready
 Serial.print(F(”         "));
 Serial.print(F("Button State:   “));
 Serial.print(buttonState);                        //printing out button State in serial monitor
 Serial.print(F(”         "));
 Serial.print(F("Photo State:    “));
 Serial.print(F(”         "));
 Serial.println(photoReading);




I don't know if your code works, but the flow is clean and I can see what you are trying to do.

I really appreciate your tips, thanks for taking the time to look!

Just FYI, you could remove the 10K pullup on the button switch and instead enable the internal pullup with

pinMode(button, INPUT_PULLUP);

There is absolutely nothing wrong with the way that it is wired now, but The internal pullups are free and commonly used.

I updated the code a little bit. The problem im having is that only the Blue LED is flashing. and its not reading the button state. when i watch the serial monitor ButtonState: is always 1 even when i press the button.

const uint8_t redLedPin=6;
const uint8_t whiteLedPin=7;
const uint8_t blueLedPin=10;
const uint8_t button=7;
const uint8_t potPin=A0;
const uint8_t photoPin=A1;
int photoReading;
int potReading;        //raw value from pot
int mapped;            //mapped value from pot
boolean buttonState;   // name for button state(true or false)
int blinkTime = 150;   // my mapped value, its easier to change it here
int photoSet = 20;     // Set where you want the Photo sensor to turn lights on here.
void setup() {
  Serial.begin(9600);                  //begin serial monitor
  pinMode(redLedPin, OUTPUT);         //set red LED pin as output
  pinMode(whiteLedPin, OUTPUT);      //set white LED pin as output
  pinMode(blueLedPin, OUTPUT);      //set blue LED pin as output
  pinMode(potPin, INPUT);          //set potPin as input
  pinMode(button, INPUT_PULLUP);         //set button pin as input
  pinMode(photoPin, INPUT);
}
void loop() {
  potReading = (analogRead(potPin));                   // Getting pot ready
  photoReading = (analogRead(photoPin));               // getting photocell ready
  Serial.print(F("Pot reading:  "));                      // printing out value so i can see it
  mapped = map (potReading, 0, 1023, 10, blinkTime);
  Serial.print( mapped);                               // mapping out value of pot for use in seconds
  buttonState = digitalRead (button);                  // getting button ready
  Serial.print(F("         "));
  Serial.print(F("Button State:   "));
  Serial.print(buttonState);                           //printing out button State in serial monitor
  Serial.print(F("         "));
  Serial.print(F("Photo State:    "));
  Serial.print(F("         "));
  Serial.println(photoReading);
  if (buttonState == false || photoReading <= photoSet ) {  //basically going to flash like a cop car while you hold down the button or if there is not enough light
    digitalWrite(redLedPin, HIGH);
    delay(mapped);                                       //pot controls the speed at which the leds flash
    digitalWrite(redLedPin, LOW);
    delay(mapped);
    digitalWrite(whiteLedPin, HIGH);
    delay(mapped);
    digitalWrite(whiteLedPin, LOW);
    delay(mapped);
    digitalWrite(blueLedPin, HIGH);
    delay(mapped);
    digitalWrite(blueLedPin, LOW);
    delay(mapped);

  }



}

const uint8_t whiteLedPin=7;
const uint8_t button=7;

You ned to decide, button or led on pin 7.

Gabriel_swe: const uint8_t whiteLedPin=7; const uint8_t button=7;

You ned to decide, button or led on pin 7.

Gabriel_swe: const uint8_t whiteLedPin=7; const uint8_t button=7;

You ned to decide, button or led on pin 7.

ahhh must have forgot to edit code after moving things around... thank you! still only the blue LED blinks... could i have burned out the LEDs this way? I had this working last night. Im not sure what happened.

I FIXED ITTTT!!! it was just a loose lead from the power haha. My first little project finally works! thanks for all the tips and help. on to figure out more complicated things.

pert:

  • Always do Tools > Auto Format on your code. If you’re using the Arduino Web Editor then you’re SOL on that. You could use an external tool such as Artistic Style, which is what the standard IDE uses.
  • Remove unnecessary blank lines from your code before posting to the forum. One or two to separate code into logical sections is fine but large spaces for no reason just make for more scrolling.
  • Quit this weird thing you’re doing with the comment spacing. My advice is just to do a single tab to separate the comments from the code. Some people try to make it all pretty by lining all the comments up but it just seems like more work than it’s worth to me.
  • Use a consistent style. In some of your code you have a space between the function name and the parentesis, other places no space. When there is no functional difference you are free to make your own choices but it’s generally considered best practice to follow existing style conventions. Thus I would recommend following the style consistently used in official Arduino example code. Some people have different preferences and that’s fine too, the important thing is to be consistent.
  • Whenever you have a variable that’s value shouldn’t change (such as your pin variables) use the const keyword: https://www.arduino.cc/en/Reference/Const
  • Generally you should use the F() macro for string literals as you use in your Serial prints. This causes them to be placed in the more plentiful flash memory instead of SRAM. This is not an absolute rule because there are times when it is better not to use the F() macro. You may actually have a shortage of flash and extra SRAM, you may be more concerned with speed than memory usage and in that case the code will be faster without the F() macro. In your code you probably have plenty of free memory and are not concerned with speed so it’s not really a big deal either way but it can become very important with more complex projects.
  • Don’t use delay. Please study File > Examples > 02.Digital > BlinkWithoutDelay and the associated tutorial page until you completely understand it. This is very important.

could you possibly help me? I tried to incorporate the no delay blink sketch into my code but i cant seem to wrap my head around how i could used the mapped value from my pot to control the rate of blinks.Right now when i click the button or wave my hand over the photoresistor the led just stays on until i either click the button or wave my hand back over it.

 const uint8_t redLedPin = 6;
const uint8_t whiteLedPin = 7;
const uint8_t blueLedPin = 10;
const uint8_t button = 3;
const uint8_t potPin = A0;
const uint8_t photoPin = A1;
int ledState = LOW;
unsigned long previousMillis = 0;
int blinkTime = 250;
int photoReading;
int potReading;        //raw value from pot
int mapped;            //mapped value from pot
boolean buttonState;   // name for button state(true or false)
const long interval = 1000;
int photoSet = 50;   // Set where you want the Photo sensor to turn lights on here.
void setup() {
  Serial.begin(9600);                  //begin serial monitor
  pinMode(redLedPin, OUTPUT);         //set red LED pin as output
  pinMode(whiteLedPin, OUTPUT);      //set white LED pin as output
  pinMode(blueLedPin, OUTPUT);      //set blue LED pin as output
  pinMode(potPin, INPUT);          //set potPin as input
  pinMode(button, INPUT);         //set button pin as input
  pinMode(photoPin, INPUT);
}
void loop() {
  potReading = (analogRead(potPin));                   // Getting pot ready
  photoReading = (analogRead(photoPin));               // getting photocell ready
  Serial.print(F("Pot reading:  "));                   // printing out value so i can see it
  mapped = map (potReading, 0, 1023, 10, blinkTime);
  Serial.print( mapped);                               // mapping out value of pot for use in seconds
  buttonState = digitalRead (button);                  // getting button ready
  Serial.print(F("         "));
  Serial.print(F("Button State:   "));
  Serial.print(buttonState);                           //printing out button State in serial monitor
  Serial.print(F("         "));
  Serial.print(F("Photo State:    "));
  Serial.print(F("         "));
  Serial.println(photoReading);
  unsigned long currentMillis = millis();
  if (buttonState == false || photoReading <= photoSet ) {  //basically going to flash like a cop car while you hold down the button or if there is not enough light
    if (currentMillis - previousMillis >= interval) {
      previousMillis = currentMillis;
      if (ledState == LOW) {
        ledState = HIGH;
      } else {
        ledState = LOW;
      }
      digitalWrite(redLedPin, ledState);
      digitalWrite(whiteLedPin, ledState);
      digitalWrite(blueLedPin, ledState);
    }
  }
}

I think this is how I would like my code to look like when I do my projects. Again just my opinion, take it with a grain of salt.

// Define input
const uint8_t button = 3;
const uint8_t potPin = A0;
const uint8_t photoPin = A1;

//Define output
const uint8_t redLedPin = 6;
const uint8_t whiteLedPin = 7;
const uint8_t blueLedPin = 10;

//Define variable
const int blinkTime = 250;
const int photoSet = 50;

void setup() {
  init_Serial();
  init_Input();
  init_Output();
}

void loop() {
  static boolean ledState = LOW;
  unsigned long timeNow = millis();
  static unsigned long timePrevious = 0;
  int potReading = analogRead ( potPin );
  int photoReading = analogRead ( photoPin );
  boolean buttonState = digitalRead (button);
  int mapped = map (potReading, 0, 1023, 10, blinkTime);
  if ( !buttonState || photoReading <= photoSet) {
    if ( timeNow - timePrevious >= mapped ) {
      timePrevious = timeNow;
      ledState = !ledState;
    }
  }
  else {
    timePrevious = timeNow;
    ledState = LOW;
  }
  update_Display ( mapped, buttonState, photoReading);
  update_Output ( ledState );
}

void init_Serial() {
  Serial.begin ( 9600 );
}

void init_Input() {
  pinMode(potPin, INPUT);
  pinMode(photoPin, INPUT);
  pinMode(button, INPUT);
  digitalWrite (button, HIGH);
}

void init_Output() {
  pinMode(redLedPin, OUTPUT);
  pinMode(whiteLedPin, OUTPUT);
  pinMode(blueLedPin, OUTPUT);
}

void update_Output( boolean outputState ) {
  digitalWrite(redLedPin, outputState);
  digitalWrite(whiteLedPin, outputState);
  digitalWrite(blueLedPin, outputState);
}

void update_Display ( int v1, boolean v2, int v3 ) {
  Serial.print(F("Pot reading:  "));
  Serial.print( v1);
  Serial.print(F("\tButton State:   "));
  Serial.print(v2);
  Serial.print(F("\tPhoto State:    "));
  Serial.println(v3);
}

Ashraf_Zolkopli:
I think this is how I would like my code to look like when I do my projects. Again just my opinion, take it with a grain of salt.

// Define input

const uint8_t button = 3;
const uint8_t potPin = A0;
const uint8_t photoPin = A1;

//Define output
const uint8_t redLedPin = 6;
const uint8_t whiteLedPin = 7;
const uint8_t blueLedPin = 10;

//Define variable
const int blinkTime = 250;
const int photoSet = 50;

void setup() {
  init_Serial();
  init_Input();
  init_Output();
}

void loop() {
  static boolean ledState = LOW;
  unsigned long timeNow = millis();
  static unsigned long timePrevious = 0;
  int potReading = analogRead ( potPin );
  int photoReading = analogRead ( photoPin );
  boolean buttonState = digitalRead (button);
  int mapped = map (potReading, 0, 1023, 10, blinkTime);
  if ( !buttonState || photoReading <= photoSet) {
    if ( timeNow - timePrevious >= mapped ) {
      timePrevious = timeNow;
      ledState = !ledState;
    }
  }
  else {
    timePrevious = timeNow;
    ledState = LOW;
  }
  update_Display ( mapped, buttonState, photoReading);
  update_Output ( ledState );
}

void init_Serial() {
  Serial.begin ( 9600 );
}

void init_Input() {
  pinMode(potPin, INPUT);
  pinMode(photoPin, INPUT);
  pinMode(button, INPUT);
  digitalWrite (button, HIGH);
}

void init_Output() {
  pinMode(redLedPin, OUTPUT);
  pinMode(whiteLedPin, OUTPUT);
  pinMode(blueLedPin, OUTPUT);
}

void update_Output( boolean outputState ) {
  digitalWrite(redLedPin, outputState);
  digitalWrite(whiteLedPin, outputState);
  digitalWrite(blueLedPin, outputState);
}

void update_Display ( int v1, boolean v2, int v3 ) {
  Serial.print(F("Pot reading:  “));
  Serial.print( v1);
  Serial.print(F(”\tButton State:  “));
  Serial.print(v2);
  Serial.print(F(”\tPhoto State:    "));
  Serial.println(v3);
}

whoa that looks beautiful! there is a lot of stuff in there i have not seen yet. im gonna take some time to look this over.