# Feedback on my code

**URL:** <https://discourse.processing.org/t/feedback-on-my-code/3764>\
**Category:** Libraries\
**Created:** [September 23, 2018, 3:34pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764 "2018-09-23T15:34:50Z")\
**Posts on this page:** 8\
**Page:** 1

<div class="post-metadata">

**Author:** ![jonaspower](https://avatars.discourse-cdn.com/v4/letter/j/f17d59/32.png) [@jonaspower](https://discourse.processing.org/u/jonaspower)\
**Post date:** [September 23, 2018, 3:34pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764/1 "2018-09-23T15:34:50Z")

</div>

Hi im new to processing and i have made this program (Its a timer/alarm)  
here is the code (probably doesnt work because of the sound thing but to make it work remember to have the sound and put a file next to the code called ding.wav)

```auto
import processing.sound.*;

SoundFile file;
String audioName = "ding.wav";
String path;
String SS="S:";
String MM="M:";
String HH="H:";
int exit = 0;
int sound = 0;
int S = 0;
int M = 0;
int H = -1;
int SE = 1;
int ME = 0;
int HE = 0;
float NIn;

void setup(){
  size (400, 212);
  path = sketchPath(audioName);
  file = new SoundFile(this, path);
  NIn = 0;
}

void draw(){ 
  background(255,100,122);
  if (NIn == 1){
  } else {
  if (keyPressed){
  if (key == 't'){
  SE=61;
  ME=61;
  HE=2147483647;
  }
  }
  }
  if (SE==60){
  SE=1;
  }
  if (ME==60){
  ME=0;
  }
  if (HE==60){
  HE=0;
  }
  if (keyPressed) {
    if (key == 's' || key == 's') {
      SE+=1;
      delay(250);
    }
  }
  if (keyPressed) {
    if (key == 'm' || key == 'm') {
      ME+=1;
      delay(250);
    }
  }
  if (keyPressed) {
    if (key == 'h' || key == 'h') {
      HE+=1;
      delay(250);
    }
  }
  if (keyPressed) {
    if (key == ' ') {
      NIn+=1;
      H+=1;
    }
  }
  //5 min. = 300000
  if (H==HE){
    if (M==ME){
      if (S==SE){
        sound+=1;
        exit+=1;
      }
    }
  }
  if (keyPressed){
  if (key == ' '||key == ' '){
  Exit();
  }
  }
  
  if (sound==1){
  file.play();
  file.play();
  }
  if (S==60){
  S=0;
  M+=NIn;
  }
  if (M==60){
  M=0;
  H+=NIn;
  }
  if (NIn==1){
  S+=NIn;
  delay(1000);
  }

  {
  smooth();
  if (NIn==1){
  textSize(64);
  text(S,0,84);
  text(M,0,148);
  text(H,0,212);}
  textSize(20);
  text(SS,0,20);
  text(MM,45,20);
  text(HH,90,20);
  text(SE,17,20);
  text(ME,67,20);
  text(HE,112,20);}
}

void Exit(){
if (exit==1){
exit();}
}

```

And i really want some constructive criticisme  
ps.  
s to set how many seconds in the alarm  
m to set how many minuets in the alarm  
h to set how many hoursin the alarm  
t to switch to timer  
space to start when your done  
hold space to close alarm when its done.

---

<div class="post-metadata">

**Author:** ![haschdl](https://yyz2.discourse-cdn.com/flex036/user_avatar/discourse.processing.org/haschdl/32/1082_2.png) [@haschdl](https://discourse.processing.org/u/haschdl)\
**Post date:** [September 23, 2018, 4:36pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764/2 "2018-09-23T16:36:52Z")

</div>

First constructive criticism: make sure if you format your code when posting to forums (any forum for that matter). If you don’t know how to do it spend some time reading the pinned posts and check other examples.

Most people will not put to put time aside to help you if they get the impression that you are not making the effort yourself.

---

<div class="post-metadata">

**Author:** ![jonaspower](https://avatars.discourse-cdn.com/v4/letter/j/f17d59/32.png) [@jonaspower](https://discourse.processing.org/u/jonaspower)\
**Post date:** [September 23, 2018, 4:43pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764/3 "2018-09-23T16:43:36Z")

</div>

Ok thx is there a way i can edit 🙂

edit: Found it.

---

<div class="post-metadata">

**Author:** ![haschdl](https://yyz2.discourse-cdn.com/flex036/user_avatar/discourse.processing.org/haschdl/32/1082_2.png) [@haschdl](https://discourse.processing.org/u/haschdl)\
**Post date:** [September 23, 2018, 5:05pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764/4 "2018-09-23T17:05:02Z")

</div>

It seems you like you can invest some time learning about different data types. Read the following:

- [https://processing.org/examples/variables.html](https://processing.org/examples/variables.html)
- [https://processing.org/examples/truefalse.html](https://processing.org/examples/truefalse.html)

Based on that… a few tips:

- If you need a variable to hold two values only, like to represent only “yes” or “no”, you can use the type `boolean` =\>\>\>\> `boolean sound;`, instead of int.

- You don’t need to have all those variables. Instead of having separate variables for minutes, hours and seconds, you can use one variable to represent the time.

---

<div class="post-metadata">

**Author:** ![jonaspower](https://avatars.discourse-cdn.com/v4/letter/j/f17d59/32.png) [@jonaspower](https://discourse.processing.org/u/jonaspower)\
**Post date:** [September 23, 2018, 5:09pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764/5 "2018-09-23T17:09:13Z")

</div>

Ok Thx a bunch i really appreciate it 😁

---

<div class="post-metadata">

**Author:** ![haschdl](https://yyz2.discourse-cdn.com/flex036/user_avatar/discourse.processing.org/haschdl/32/1082_2.png) [@haschdl](https://discourse.processing.org/u/haschdl)\
**Post date:** [September 23, 2018, 5:28pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764/6 "2018-09-23T17:28:24Z")

</div>

you’re welcome! let us know about your progress and others can give more tips.

---

<div class="post-metadata">

**Author:** ![jeremydouglass](https://yyz2.discourse-cdn.com/flex036/user_avatar/discourse.processing.org/jeremydouglass/32/20_2.png) [@jeremydouglass](https://discourse.processing.org/u/jeremydouglass)\
**Post date:** [September 25, 2018, 4:38pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764/7 "2018-09-25T16:38:35Z")

</div>

@jonaspower – Some other suggestions:

1. `key == ' '||key == ' '` is checking the same thing twice

2. If you are doing long groups of repetitive textual operations you can either combine the strings before printing them or wrap them up in a function. The string approach is “combine my letter, colon, and value into a single string, then print it with one text call.” The function approach is to write your own `void textData(String label, int value, float x, float y)` which combines those repeated offsets, font sizes, etc. Then call it like this:

```auto
textData(S, SS, 0, 84);
textData(M, MM, 0, 148);
textData(H, HH, 0, 212);

```

1. Think about ways that you could use Processing’s built-in timing information, under the Time & Date section of the [Processing reference](https://processing.org/reference/) – in particular, millis.

- [millis()](https://processing.org/reference/millis_.html)
- [second()](https://processing.org/reference/second_.html)
- [minute()](https://processing.org/reference/minute_.html)
- [hour](https://processing.org/reference/hour_.html)

1. This is more esoteric, but you might find it useful. Rather than creating your own Exit with a similar name (which can be confusing because methods are not normally capitalized – perhaps call it `beforeExit`?) you can simply redefine exit and call the original with `super`, like this:

```auto
void exit(){
  if (exit==1){
    super.exit();
  }
}

```

---

<div class="post-metadata">

**Author:** ![jonaspower](https://avatars.discourse-cdn.com/v4/letter/j/f17d59/32.png) [@jonaspower](https://discourse.processing.org/u/jonaspower)\
**Post date:** [September 25, 2018, 5:35pm UTC](https://discourse.processing.org/t/feedback-on-my-code/3764/8 "2018-09-25T17:35:16Z")

</div>

I see. Thanks a lot 😁