Peer-Review HW-08 for @MalcolmNSB

[zeeva85]

Peer-Review HW-08 for @MalcolmNSB

Topic	Excellent	Satisfactory	Needs Work
Coding style		:heavy_check_mark:
Coding strategy	:heavy_check_mark:
Presentation: graphs	:heavy_check_mark:
Presentation: tables	:heavy_check_mark:
Achievement, creativity	:heavy_check_mark:
Ease of access	:heavy_check_mark:

Remarks

Elaborate on above, esp. for "needs work."

Straight forward and clean coding style. There should be some comments in your work flow. There isnt a single line of comment in your app. Although I understood it well and easily it may not apply for everyone, so some form documentation is required. It does not need to be line for line, this is especially true for trivial things but for something like "this does this" would have been sufficient in my opinion. Also a little more separtion makes it more readable (see below)

The coding stratergy was very clean and concise, I like it. You kept it fresh and simple. I provided a tip for hope you find it useful

The graphs were okay, cant really say much since I it was the standard one provided when you made your repo. The table however was good pairing with your sort by options.

The effort and creativity was indeed sufficent, in my humble opinion there could have been a little more exploration on your side.

Your content, the information you provided, and your app was very accesible and plesant to review. When I cloned you repo and ran your app, everthing worked perfectly without me needing to do anthing. Great job on keeping it simple and clean.

Some specific praise?

Very straight forward and clean assignment, very easy to follow and because it didnt have many dependancy it was very essy to run.

WELL DONE !! 👍.

Something I learned? Something I know and that you, my peer, might like to know because it is relevant to something you struggled with.

I did a different app not the bcl one, check out my repo below if your interested, so I never knew the rendertext option on my shinny app. Since I skipped the class app, I learned that from reviewing your app and Thank you very much !!

Specific constructive criticism?

1. Definately include some comments in your codes. As per the separation some separation for readability-

You did this 

filtered <- reactive({
    if (is.null(input$countryInput)) {
      return(NULL)
    }  
    bcl_modified <- bcl %>%
      filter(Price >= input$priceInput[1],
             Price <= input$priceInput[2],
             Type == input$typeInput,
             Country == input$countryInput
      ) %>% 
      select(c(-Country, -Type))
    if(nrow(bcl_modified) == 0){
      return(NULL)
    }
    if(input$sortInput == "Price"){
        return(arrange(bcl_modified, Price))
    }
    else if(input$sortInput == "Alcohol content"){
      return(arrange(bcl_modified, Alcohol_Content))
    }
    else if(input$sortInput == "Name"){
      return(arrange(bcl_modified, Name))
    }
    else if(input$sortInput == "Sweetness"){
      return(arrange(bcl_modified, Sweetness))
    }
    else{
      return(bcl_modified)
    }

  })

# That could have used a space before the `if` statement.

             Type == input$typeInput,
             Country == input$countryInput
      ) %>% 
      select(c(-Country, -Type))

    if(nrow(bcl_modified) == 0){
      return(NULL)
    }

The space seems like a small thing but it separates the workflow so it would be easier to follow since its compartmentalized and makes it more readable. If you look at that lines of codes there is 29 lines of code that makes it seem like its doing one function, where indeed the first part of the fucntion with displaying the reactive table, the second if else statement is for sorting them.

2. A more elegant coding stratergy for your radio buttons:-

      radioButtons("sortInput", "Sort by", choices = names("Name", "Alcohol content", "Price",  "Sweetness"), selected = NULL)

      vs

      radioButtons("sortInput", "Sort by", choices = names(bcl)[-1:-3], selected = NULL)     

2. When you load your library(tidyverse) without supressing messages you would typically see the message below:-

library(tidyverse)

── Attaching packages ──────────────────────────────────── tidyverse 1.2.1 ──
✔ ggplot2 3.1.0     ✔ purrr   0.2.5
✔ tibble  1.4.2     ✔ dplyr   0.7.8
✔ tidyr   0.8.2     ✔ stringr 1.3.1
✔ readr   1.1.1     ✔ forcats 0.3.0
── Conflicts ─────────────────────────────────────── tidyverse_conflicts() ──
✖ dplyr::filter() masks stats::filter()
✖ dplyr::lag()    masks stats::lag()

# You could have gotten away with this

library(shiny)
library(tidyverse)
library(DT)

loading tidyverse attaches the aforementioned packages above, that includes "ggplot2", "dplyr", "tibble", "purrr", "stringr". You could have gotten away with 2 extra lines lesser when you used the use library(tidyverse).

"Tidyverse" package is a suite of packages from Hadley that was precompiled on frequently used packages for data manipulation and visualization. It is easier to just load the Tidyverse suite rather than the individual packages.

3. A nice included option for country separation or all countries. I managed to find a package that gives an additional options of multi-country options. If your interested in developing that option using that widget you can check it here.

Kudos🥇, well done and Thank you @MalcolmNSB

Seevasant Indran