Writing cleaner PHP code to add additional options I have a contact form that will perform validation on the server. Right now the form checks if the fields are empty or not. My goal is to expand it following:

Check the fields for particular format, if it is a proper email format, phone number format, etc
Set the field as required or not

My form is as below, I appreciate any suggestions on how to clean it up and make it easier to adjust and overall cleaner.

<?php

ini_set("log_errors", 1);
ini_set("error_log", "php-error.log");
error_log( "Errors" );

include "recaptcha.php";
include 'email.php';
use PHPMailer\PHPMailer\PHPMailer;
use PHPMailer\PHPMailer\Exception;

require 'Exception.php';
require 'PHPMailer.php';
require 'SMTP.php';

$errors         = array();      // array to hold validation errors
$data           = array();         // array to pass back data

// validate the variables ======================================================
    // if any of these variables don't exist, add an error to our $errors array

    if (empty($_POST['firstName']))
        $errors['firstName'] = 'First Name is required.';

    if (empty($_POST['lastName']))
        $errors['lastName'] = 'Last Name is required.';

    if (empty($_POST['companyName']))
        $errors['companyName'] = 'Company Name is required.';

    if (empty($_POST['companyAddress']))
        $errors['companyAddress'] = 'Company Address is required.';

    if (empty($_POST['city']))
        $errors['city'] = 'City is required.';

    if (empty($_POST['state']))
        $errors['state'] = 'State is required.';

    if (empty($_POST['emailAddress']))
        $errors['emailAddress'] = 'Email Address is required.';

    if (empty($_POST['comment']))
        $errors['comment'] = 'Comment is required.';

    if (empty($_POST['g-recaptcha-response']))
        $errors['recaptcha'] = 'Captcha is required.';

// return a response ===========================================================

    // Check ReCaptcha Validation

    if(!validateRecaptcha($secret, $_POST['g-recaptcha-response'], $_SERVER["REMOTE_ADDR"]))
    {
        $errors['recaptcha'] = 'Captcha is required.';
    }

    if ( ! empty($errors)) {

        // if there are items in our errors array, return those errors
        $data['success'] = false;
        $data['errors']  = $errors;
    } else {

        // if there are no errors process our form, then return a message

        // DO ALL YOUR FORM PROCESSING HERE
        // THIS CAN BE WHATEVER YOU WANT TO DO (LOGIN, SAVE, UPDATE, WHATEVER)

        // show a message of success and provide a true success variable
        $data['success'] = true;
        $data['message'] = 'Success!';
    }

    // return all our data to an AJAX call
    echo json_encode($data);

//Begin of send email
$mail = new PHPMailer;

//Enable SMTP debugging.
$mail->SMTPDebug = 3;
//Set PHPMailer to use SMTP.
$mail->isSMTP();
//Set SMTP host name
$mail->Host = "smtp.server.com";
//Set this to true if SMTP host requires authentication to send email
$mail->SMTPAuth = true;
//Provide username and password
$mail->Username = "user@server.com";
$mail->Password = "super_secret_password";
//If SMTP requires TLS encryption then set it
$mail->SMTPSecure = "tls";
//Set TCP port to connect to
$mail->Port = 587;

$mail->From = $emailAddress;

$mail->addAddress("name@server.com", "Recepient Name");

$mail->isHTML(true);

$mail->Subject = "Subject Text";
$mail->Body = $emailBody;

if(!$mail->send())
{
  $data['success'] = false;
  $data['errors']  =  "Mailer Error: " . $mail->ErrorInfo;
}
else
{
    $data['message'] = 'Success!';
}

?>

General inconsistency
Basically your code is borderline off topic. Although it could work, it is not a working code in the sense a programmer could take it; but rather a set of copy-pasted code blocks loosely (and wrongly) connected to each other.
The main problem is inconsistent code structure. You have $data['success'] set at the end of the script but... it goes nowhere.
So it basically should be
if ($errors) {
    $data['success'] = false;
    $data['errors']  = $errors;
} else {
    $mail = new PHPMailer;
    // the rest or phpmailer code goes on
    if(!$mail->send())
    {
        $data['success'] = false;
        $data['errors']  =  ["Mailer Error: " . $mail->ErrorInfo];
    }
    else
    {
        $data['success'] = true;
        $data['message'] = 'Success!';
    }
}
// return all our data to an AJAX call
echo json_encode($data);

Note that another error has been fixed. $data['errors'] should be an array, but in case of phpmailer error you were sending a string.
Configuration options
error_log( "Errors" );

Always use the absolute filesystem path to a log file or you'll end up with log files in the every directory.
Also, it's better to configure such settings on the server level, for some errors may occur before the file gets parsed completely. Or at least put these settings into a configuration file and include it in all your php scripts.
Autoload
Instead of requiring files manually, consider using PSR-4 autoload and Composer. It's really easy, you just have to install Composer, then run two commands in console
php composer.phar require google/recaptcha
php composer.phar require phpmailer/phpmailer

and then add just a single require line into your code
require __DIR__ . '/vendor/autoload.php';

and all classes will be loaded automatically!
Validation and sanitization
You may want to verify whether the email address is valid.
if (empty($_POST['emailAddress'])) {
    $errors['emailAddress'] = 'Email Address is required.';
} else {
    $emailAddress = filter_var($errors['emailAddress'], FILTER_SANITIZE_EMAIL);
    if (!filter_var($emailAddress, FILTER_VALIDATE_EMAIL)) {
        $errors['emailAddress'] = 'Email Address is invalid.';
    }
}

and also sanitize the email body,
$emailBody = htmlspecialchars($_POST['comment']);


Setting the implementation of an abstract class based on settings in config file using smart pointers I want to choose the implementation of an abstract class based on settings from a config file. Is this the proper way of doing that?

I am asking because I am having doubts about using std::make_unique inside  the if scope. Will the object created with std::make_unique still be valid outside of the if scope?

#include <iostream>
#include <memory>

// Base class for all position systems
class PositionSystem {
public:
    virtual int getX() const = 0;
};

// GPS implementation
class GPS: public PositionSystem {
public:
    int getX() const {return 42;};
};

// Galileo implementation
class Galileo: public PositionSystem {
public:
    int getX() const {return 21;};
};

int main(){
    // Settings are read from config file
    // and stored in a smiliar struct
    struct {std::string position_system = "Galileo";} settings;

    // Declare position system
    std::unique_ptr<PositionSystem> ps;

    // Choose implementation based on settings
    if(settings.position_system == "GPS"){
        ps = std::make_unique<GPS>(GPS());
    }
    else if (settings.position_system == "Galileo"){
        ps = std::make_unique<Galileo>(Galileo());
    }

    // Use interface
    std::cout << ps->getX() << '\n';

    return 0;
}

When you have a derived type deleted via a pointer to the base class you need to define a virtual destructor. This also applies when you are using unique_ptr.

// Base class for all position systems
class PositionSystem {
public:
    virtual ~PositionSystem() {}    // You need this or your object will
                                    // not be correctly destroyed.
    virtual int getX() const = 0;
};

You don't need to create an objet and copy it.

    ps = std::make_unique<GPS>(GPS());
                           //  ^^^^^    This creates a automatic object
                           //           When make_unique calls new
                           //           this automatic object is copy constructed
                           //           into the dynamic object.

Simply do this:

    ps = std::make_unique<GPS>();

You have the correct answer. Its an if/switch/command patter that you use to create the correct object. But you don't need to use a unique pointer. You could use a reference rather than a unique_ptr (but a unique_ptr is simpler).

But rather than do it in main I would move this work into a factory specifically for this job.

std::unique_ptr<PositionSystem> ps = positionSystemFactory(settings);

Then you just need to write the factory method:

std::unique_ptr<PositionSystem> positionSystemFactory(Settings const& settings)
{
    // Choose implementation based on settings
    if(settings.position_system == "GPS"){
        return std::make_unique<GPS>();
    }
    else if (settings.position_system == "Galileo"){
        return std::make_unique<Galileo>();
    }
    throw Failure;
}

  am having doubts about using std::make_unique inside the if scope.

Yes it will work as expected.

else if (settings.position_system == "Galileo"){
        ps = std::make_unique<Galileo>(Galileo());
    }

Even though the temp object created by make_unique() will be destroyed when it goes out of scope; this object is assigned to the variable ps where its content is maintained.

How this works: The temp object invokes the templatized move constructor on ps thus resulting in the contained pointer being moved from the temp object into ps (which takes ownership away from the temp object). When the temp object is destroyed it no longer owns the pointer and thus does not delete it.


Animating the appearance of a title with CSS To sum up my project was about making cool animation of titles appearance with an effect of a colored block coming from one side while expanding and disappearing from the others side while contracting and let the text appear.

My goal was to achieve something simple, that can be used again easily on different elements just by giving them the right classes.

I use only CSS for the animation to improve load time.

I absolutely want the animation to automatically adapt to the size of the text it contains.

Problems I encountered:

The width animation is easy from left to right but not from right to left because there's no origin define in CSS 'animation' (like in 'transform') to solve the problem I use two elements:

First one with position absolute, left:0
Second one with position absolute, right:0

Each one makes only one part of the animation. I find it a bit sad to use two elements to pretend the animation of only one element but I haven't figured out a different way of doing this.

html, body{
 margin: 0;
 padding: 0;
 min-width: 1200px;
 width: 100%;
 height: 100%;
 background-color: #FE4D3D;
 font-family: arial;
}

#container{
 margin-left: 10px;
 display: inline-block;
}

.animated-block-from-left{
 position: absolute;
 top:0;
 left:0;
 z-index: 2;
 height: 100%;
   width: 0%;
   animation: grandir 1s ease ;

}

@keyframes grandir{
 0%{width: 0%;}
 100%{width: 100%;}
}

.animated-block-from-right{
 position: absolute;
 top:0;
 right:0;
 z-index: 2;
 height: 100%;
   width: 0%;
   animation: retrecir 1s 1s ease ;

}

@keyframes retrecir{
 0%{width: 100%;}
 100%{width: 0%;}
}

.yellow{
 background-color: #FFFE02;
}

h1{
 text-align: center;
 padding: 20px;
 box-sizing: border-box;
 position: relative;
 font-size: 80px;
 color: transparent;
 animation: textappear 0.1s 1s forwards;
 z-index: 1;
}

@keyframes textappear{
 100%{color: #FFFE02;}
}

  HELLO JOHN !

This can be achieved with one element and a CSS pseudo-element.

The html

<element class="animate right">Hello world!</element>

Add the animate class to any element that will have this animation
applied to it.
Add a left or right class in order to specify which direction the animation should travel

The pseudo-element

.animate::before {
    content: '';
    position: absolute;
    background: #ff0;
    width: 100%;
    top: 0;
    bottom: 0;
}

The pseudo element is positioned absolute, 100% in width and stretched from top to bottom with top: 0 and bottom: 0.

There are two animations, left and right which have their own class:

.right::before {
    animation: sliderRight 2s forwards;
}

.left::before {
    animation: sliderLeft 2s forwards;
}

The animate class

Any element that you want to add this animation to will be given this class:

.animate {
    position: relative;
    color: transparent;
    padding: 10px;
    overflow: hidden;
    display: inline-block;
    margin: 20px;
    animation: textappear .1s .5s forwards;
}

The position: relative will make the position: absolute ::before pseudo-element child position itself relevant to the .animate parent.
The element must be inline otherwise the wiping animation will take the entire width of the screen (unless you use a width)
The overflow: hidden will cut off the animation correctly

The sliding animations

@keyframes sliderRight {
    0% {
        right: 100%; /*Push the element 100% from the right side*/
    }

    100% {
        right: -100%; /*Pull the element from left to right*/
    }
}

@keyframes sliderLeft {
    0% {
        left: 100%; /*Push the element 100% from the left side*/
    }

    100% {
        left: -100%; /*Pull the element from right to left*/
    }
}

Complete example

.animate {
  position: relative;
  color: transparent;
  padding: 10px;
  overflow: hidden;
  display: inline-block;
  margin: 20px;
  animation: textappear .1s .5s forwards;
}
.animate::before {
  content: '';
  position: absolute;
  background: #ff0;
  width: 100%;
  top: 0;
  bottom: 0;
}
.right::before {
  animation: sliderRight 2s forwards;
}
.left::before {
  animation: sliderLeft 2s forwards;
}
@keyframes textappear {
  100% {
    color: #FFFE02;
  }
}
@keyframes sliderRight {
  0% {
    right: 100%;
  }
  100% {
    right: -100%;
  }
}
@keyframes sliderLeft {
  0% {
    left: 100%;
  }
  100% {
    left: -100%;
  }
}
/*Demo only*/

.animate {
  white-space: nowrap;
}
h1.animate {
  font-size: 5em;
}
body {
  margin: 0;
  padding: 0;
  background-color: #FE4D3D;
  font-family: arial;
}

Hello World!

Hello World!


Aligning page elements with window size The following is a series of functions to ensure that various elements on a page line up no matter what window size or when the window is resized. However I'm not sure my code is very concise as I am using the same function on three different event handlers. I read that function expressions aren't "hoisted" so I can't get the proper values on $(document).ready but I'm not sure how I could declare them with out making the code really messy and repetitive.

I've made a jsfiddle with the complete HTML and CSS.

 function captHeight(){
    var figCaption = $('.cap-bot').find($('figcaption'));
    var captionHeight = Math.max.apply(null, $('figcaption').map(function ()
      {
          return $(this).height();
    }).get());

    $('.cap-bot').css({
      'overflow': 'visible'
      });
      figCaption.css({
      'position': 'relative',
      'opacity': '1',
      'bottom': '0',
      'min-height': captionHeight+ 20 + "px" //to account for padding
      });
}

function sameHeight(){
if($('.find-height').length > 0){
    var foundHeight= $('.find-height').height();
    var gaveHeight= $('.give-height').height();

    if (gaveHeight > foundHeight){
      captHeight();
    }

    $('.give-height').css({
      'min-height': foundHeight
    });
  }
}

$(document).ready(function(){
  sameHeight();
});

$(window).on('resize', function(){
  sameHeight();
});

$(window).on('scroll', function(){
  sameHeight();
});

I read that function expressions aren't "hoisted"

Yes. See var functionName = function() {} vs function functionName() {}.

  so I can't get the proper values on $(document).ready

Why that? Unless the assignment to sameHeight did happen after the $(document).ready call, you can easily get the function.

  but I'm not sure how I could declare them with out making the code really messy and repetitive.

You already have declared them, they are hoisted and everything is fine. However, even if you did use function expressions:

var captHeight = function() {
    …
};
var sameHeight = function() {
    …
};
$(document).ready…

then everything would have been fine. Notice that, as @Flambino mentioned already in the comments, you can shorten the part where you install the event listener to

$(document).ready(sameHeight);

$(window).on('resize', sameHeight)
         .on('scroll', sameHeight);


Check if integer is a palindrome in base 10 Here is the solution->
//Write a program to check the number whether it is palindrome or not
#include <stdio.h>
int
main(void){

//Put variables for the further proceed
int number, quotient=1, remainder,i=0;

//To declare a character array
char text[100];

//To show the message to the user
printf("Enter an integer number :");

//Taking input from the user
scanf("%d",&number);

//For finding escape the integer number in the reverse order specifically
int number_update=number;

//To find out the integer number in the reverse order
  while(quotient!=0){
  quotient=number_update/10;
  remainder=number_update%10;
  number_update=quotient;
  text[i] = remainder + '0';//Converts integer to character and store to the array
  i++;
  }
 //Converts the string to a whole integer
  int result_of_reverse=atoi(text);

  //Check the result of reverse order with the given integer number
  if(result_of_reverse==number){

    //To display the result
    printf("This is a palindrome number");
  }
  else{

    //To display the result
    printf("This is not a palindrome number");
  }

  }

I want to achieve: How can I simplify my solution? Is there any better strategy should I follow to solve these types of problems?

Too many comments, and they do not serve any purpose.

Use all necessary #includes. atoi requires #include <stdlib.h>.

Separate IO from the business logic. Constructing the reversed number definitely belongs to a function.

There is no need to convert the number into a textual representation. Consider building the reversed number as you compute the digits of the original:
int reverse (int number)
{
    int reversed = 0;
    while (number != 0) {
        int last_digit = number % 10;
        number /= 10;
        reversed *= 10;
        reversed += last_digit;
    }
    return reversed;
}

As a side effect, many variables (quotient, remainder, number_update, i) disappear.


Onclick radio buttons to change background color We are displaying two radio buttons in page. Click on Radio buttons, it will change background color.  Is there any way to improve the quality of the code?

html,
body {
  width: 100%;
  height: 100%;
  margin: 0;
  padding: 0;
  text-align: center;
}

body {
  box-sizing: border-box;
  padding-top: 180px;
  background: #ffffff;
}

input {
  display: none;
}

.button {
  display: inline-block;
  position: relative;
  width: 50px;
  height: 50px;
  margin: 10px;
  cursor: pointer;
}

.button span {
  display: block;
  position: absolute;
  width: 50px;
  height: 50px;
  padding: 0;
  top: 50%;
  left: 50%;
  -webkit-transform: translate(-50%, -50%);
  -ms-transform: translate(-50%, -50%);
  -o-transform: translate(-50%, -50%);
  transform: translate(-50%, -50%);
  border-radius: 100%;
  background: #eeeeee;
  box-shadow: 0 2px 5px 0 rgba(0, 0, 0, 0.26);
  transition: ease .3s;
}

.button span:hover {
  padding: 10px;
}

.orange .button span {
  background: #FF5722;
}

.amber .button span {
  background: #FFC107;
}

.layer {
  display: block;
  position: absolute;
  width: 100%;
  height: 100%;
  top: 0;
  left: 0;
  background: transparent;
  /*transition: ease .3s;*/
  z-index: -1;
}

.orange input:checked ~ .layer {
  background: #F4511E;
}

.amber input:checked ~ .layer {
  background: #FFB300;
}

  <input type="radio" name="color" value="orange">

  <input type="radio" name="color" value="amber">

The code looks pretty good. I only see a couple CSS rules that appear to be useless - for instance:

background: #eeeeee;

under the ruleset:

.button span {

But that style is overridden by the background styles under the <label> tags (i.e. .orange .button span and .amber .button span). The only reason I could think that would be needed is if you have other HTML not included in the example above that would have other colors...

What is the goal of the styles for html? I read the answers to Styling the <html> element in CSS? and it leads me to believe it is an attempt to avoid scrollbars, but maybe that is incorrect.

And is it really necessary to specify

background: #ffffff;

for the body tag? Maybe there are browser settings I am unaware of that allow a user to have a default background color other than that??


How to make this query better (e.g. avoid repeating subquery in a query)? Stack Exchange provides access to query their MS SQL databases. They have a table Posts where each post is either a question or an answer (see the schema on right hand side here).
I saw a query for questions answered by more than three 50k users:
SELECT DISTINCT Q.Id AS [Post Link], Q.AnswerCount
FROM Posts Q
JOIN Posts A1 on A1.ParentId = Q.Id
  AND A1.OwnerUserId IN (SELECT TOP ##TopN## Id FROM Users ORDER BY Reputation DESC)
JOIN Posts A2 on A2.ParentId = Q.Id
  AND A2.OwnerUserId IN (SELECT TOP ##TopN## Id FROM Users ORDER BY Reputation DESC)
  AND A1.Id != A2.Id
JOIN Posts A3 on A3.ParentId = Q.Id
  AND A3.OwnerUserId IN (SELECT TOP ##TopN## Id FROM Users ORDER BY Reputation DESC)
  AND A3.Id != A1.Id AND A3.Id != A2.Id
WHERE Q.AnswerCount = 3

I was wondering how to improve the query (balance between efficiency and readability)? For example:

Is it possible to avoid repeating the subquery SELECT TOP ##TopN## Id FROM Users ORDER BY Reputation DESC three times, and instead just have it once?

Will it be more  efficient if switching the order of filtering out questions with less than three answers and checking if  a question is answered by more than three 50k users?

The query title is

Questions answered by more than 3 50k users

But that doesn't describe what the query actually does. The query selects questions with three answers given by three distinct top n rated users. They happen to have more than 50k rep on that site, but that's a mere coincidence.
That can also be done by this query, containing only 1 subquery to get the top rated users:
SELECT Q.Id AS [Post Link], Q.AnswerCount
FROM Posts Q
WHERE Q.AnswerCount = 3
AND
(
  SELECT COUNT( DISTINCT A.OwnerUserId)
  FROM Posts A
  WHERE A.ParentId = Q.Id
  AND A.OwnerUserId IN
  (
    SELECT TOP ##TopN## Id
    FROM Users
    ORDER BY Reputation DESC
  )
) = 3

For the record, the query doing what the title says would be
SELECT Q.Id AS [Post Link], Q.AnswerCount
FROM Posts Q
WHERE Q.AnswerCount >= 3
AND
(
  SELECT COUNT(DISTINCT A.OwnerUserId)
  FROM Posts A
  JOIN Users U ON A.OwnerUserId = U.Id
  WHERE A.ParentId = Q.Id
  AND U.Reputation >= 50000
) = Q.AnswerCount

Assuming that the intention was to get questions with answers from 50k users exclusively.


Adding students to rows in a classroom I'm learning Scala and functional programming. Can I make it more functional?

class Student(val firstname: String, val lastname: String) {
  override def toString: String = firstname + " " + lastname
}
class ClassRoom {
  var rows: List[Array[Student]] = List[Array[Student]]()
  var total: Int = _
  def addRow(row: Array[Student]) = {
    rows = row :: rows
    total += row.length
  }
  def print = rows.foreach(r => r.foreach(println))
}

A couple of small things to start.

var rows: List[Array[Student]] = List[Array[Student]]()

This line can be simplified slightly by doing this instead:

var rows: List[Array[Student]] = List.empty

Unless you are caching the total for performance reasons, you could calculate it as needed with something like this:

def total: Int = rows.map(_.length).sum

That would simplify addRow() to this:

def addRow(row: Array[Student]) = rows = row :: rows

One of the tenets of functional programming is the use of immutable data. ClassRoom is not very functional from this perspective: both rows and total are mutable. If you wanted to take a pure functional approach, all members would be val and functions like addRow() would return a new ClassRoom instead of modifying itself. For example:

class ClassRoom(val rows: List[Array[Student]]) {
  def total: Int = rows.map(_.length).sum
  def addRow(row: Array[Student]) = new ClassRoom(row :: rows)
  def print = rows.foreach(_ foreach println)
}


Convert a date string to a DateTime object in C# I was just wondering if my code contains defects or if there was a more efficient or general way to convert string to DateTime object. In the input, I have a string in the UTC format.

public static DateTime ParseDateTime(string dateString)
    {
        DateTime dateTime;
        if (!DateTime.TryParse(dateString, CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal, out dateTime))
        {
            if (!DateTime.TryParse(dateString, CultureInfo.CurrentCulture, DateTimeStyles.AssumeUniversal, out dateTime))
            {
                try
                {
                    dateTime = DateTime.Parse(dateString, CultureInfo.InvariantCulture, DateTimeStyles.AssumeLocal);
                }
                catch (FormatException)
                {
                    // Try to extract at least year from the string (the longest digits substring)
                    var yearMatch = Regex.Matches(dateString, @"\d{4}").Cast<Match>().FirstOrDefault();
                    if (yearMatch == null || string.IsNullOrWhiteSpace(yearMatch.Value))
                    {
                        throw;
                    }

                    // Only year really matters for Max and Min values of DateTime
                    var year = int.Parse(yearMatch.Value, CultureInfo.InvariantCulture);

                    // Try to determine what do we have (Min or Max value)
                    if (year == DateTime.MaxValue.Year)
                    {
                        dateTime = DateTime.MaxValue;
                    }
                    else
                    {
                        if (year == DateTime.MinValue.Year)
                        {
                            dateTime = DateTime.MinValue;
                        }
                        else
                        {
                            throw;
                        }
                    }
                }
            }
        }

        dateTime = dateTime.ToUniversalTime();
        return dateTime;
    }

Nesting if-blocks like you do is a way that leads to "arrow code", code that has deeply indented structures/blocks.

In your case, I will assume that you can't use the more simple 1-try TryParse(dateString, datetime) option, and that you have to try the invariant and local cultures in that order.

So, consider this code:

    DateTime dateTime;

    if (DateTime.TryParse(dateString, CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal, out dateTime))
    {
        return dateTime.ToUniversalTime();
    }

    if (DateTime.TryParse(dateString, CultureInfo.CurrentCulture, DateTimeStyles.AssumeUniversal, out dateTime))
    {
        return dateTime.ToUniversalTime();
    }

    if (DateTime.TryParse(dateString, CultureInfo.InvariantCulture, DateTimeStyles.AssumeLocal, out dateTime))
    {
        return dateTime.ToUniversalTime();
    }

Now the logic progression between your format attempts is clear. It also makes it easier to add a new attempt in there just by adding 4 lines.

If your attempts are not successful, though, your code pulls out a year. This code is broken because it does not actually return a datetime for valid years...

If the year is between min and max dates, it does not set the datetime at all...


Convert String to Integer with default value I want to convert a java.lang.String to a java.lang.Integer, assigning a default value of 0 if the String is not convertible. Here is what I came up with.  I would appreciate an assesment of this approach.

To be honest, it feels a little squirrely to me:

String strCorrectCounter = element.getAttribute("correct");
Integer iCorrectCounter = new Integer(0);
try {
    iCorrectCounter = new Integer(strCorrectCounter);
} catch (Exception ignore) { }

here is a solution :

int tryParseInt(String value) {
 try {
     return Integer.parseInt(value);
  } catch(NumberFormatException nfe) {
      // Log exception.
      return 0;
  }
}

you should catch NumberFormatException instead of exception.


indexOf() and Boyer Moore methods I have a method I created which is a custom indexOf(). This code seems ugly to me so I was wondering if anyone could recommend a cleaner and perhaps faster implementation of my method.

public static int myIndexOf(char[] str, char[] substr, int index) {
    if (str == null) {
        throw new NullPointerException();
    }
    if (substr.length > str.length) {
        return -1;
    }
    if (substr.length == 0 && index >= str.length) {
        return str.length;
    }
    if (substr.length == 0 && index >= 0) {
        return index;
    }
    if (substr.length == 0 && index < 0) {
        return 0;
    }
    if (substr.length == 0 && str.length == 0 ) {
        return 0;
    }

    int[] found = boyerMoore(str, substr);
    for (int i : found) {
        if (i >= index) {
            return i;
        }
    }
    // can not found substring after index
    return -1;
}

// Don't necessarily need any modification for this method. But for those who are interested
Boyer Moore:

public static int[] boyerMoore(char[] str, char[] substr) {
    // index array of indexes of found substrings
    // initialize
    int indexCounter = 0;
    int[] subStringIndex = new int[str.length];
    for (int c = 0; c < str.length; c++) {
        subStringIndex[c] = -1;
    }
    // character set ASCII
    int characterSet = 256;
    // initialize
    int[] charSet = new int[characterSet];
    for (int a = 0; a < characterSet; a++) {
        charSet[a] = -1;
    }
    for (int b = 0; b < substr.length; b++) {
        charSet[substr[b]] = b;
    }

    int skip;
    for (int i = 0; i <= (str.length - substr.length); i += skip) {
        //System.out.print(i);
        skip = 0;
        for (int j = substr.length - 1; j >= 0; j--) {
            if (substr[j] != str[i + j]) {
                skip = Math.max(1, j - charSet[str[i + j]]);
                    break;
            }
        }

        if (skip == 0) {
            // increment next value in String
            skip++;
            // add found substring (beginning index)
            subStringIndex[indexCounter] = i;
            indexCounter++;
            // this last bit is used if Boyer Moore stops searching after first instance is found
            // return i;

        }
    }

    return  subStringIndex;
    // This bit is used when Boyer Moore is stops after the first instance in found
    // cannot be found
    // return str.length;
}

Naming

Choose an existing idiom such as "find a needle in a haystack" and consider putting the parameters in that order.
The name index implies some generic location within haystack. However, start makes it clear why that index is significant.

The rest of my answer assumes these changes.

Validating Input

You don't need to manually throw NullPointerException for null values when accessing the length of each string will do it automatically. If not, make sure to check both strings for consistency.
Consider how to handle negative values for start, typically by taking it from the end of the haystack. You'll need to decide whether to use modulo arithmetic or constrain overly-negative values to zero.

if start < 0
    start = Math.max(0, haystack.length + start)

Logic Yoga

Combine identical prefix expressions in an if chain.

if (A && R) { X }
if (A && S) { Y }
if (A && T) { Z }

This can be simplified and optimized to

if (A) {
    if (R) { X }
    if (S) { Y }
    if (T) { Z }
}

It works for || as well, though I can't think of a recent sighting in the wild.

if (A || R) { X }
if (A || S) { Y }
if (A || T) { Z }

becomes

if (A)
    X Y Z
else
    // same as "if (A)" above

Clarify intent by using else if after blocks that should exit. Adding else lets the compiler enforce that intent when they're modified.

myIndexOf(needle, haystack, start)
    if needle is larger than haystack
        return ...
    else if needle is empty
        return ...
    else
        return ...

Extract Methods

Move generic helper methods such as constraining an index to a library. Apache Commons probably has one. This way someone doesn't have to read the code three times to infer its intended purpose (versus what it actually does). Here index makes more sense without a context.

constrain(index, size)
    return Math.max(0, Math.min(index, size))

Separate parameter checking from the raw search algorithm when either is long enough.

findBoyerMore(needle, haystack, start)
    for index in boyerMoore(needle, haystack)
        if index >= start
            return index
    return -1

Extract a method to test if needle can fit inside haystack at start.

Also, you can fail fast in the case of myIndexOf("foo", "foobar", 5) without calling boyerMoore even though foo is shorter than foobar. Take start into consideration when comparing the length of the two strings.

canFitAt(needle, haystack, start)
    return start + needle.length <= haystack.length

Dead Code

Since index >= 0 and index < 0 cannot both fail, if (substr.length == 0 && str.length == 0) cannot succeed and can be removed.

End Result

Combining all of the above gives us a much-simplified method.

myIndexOf(needle, haystack, start)
    if canFitAt(needle, haystack, start)
        return findBoyerMoore(needle, haystack, start)
    else
        return constrain(start, haystack.length)


autoloader class fine tune to be usable in every project I'm open to any advice to help improve this code for general use.

/**
*
* This code was created by Joshua Getner.
* @license http://opensource.org/licenses/bsd-license.php BSD
*
*
*/

namespace Blacksmith;

class Autoloader
{

    // an array that maps classnames to there filepaths
    protected static $classes = array();

   /**
    * register your classnames and class paths to the autoloader.
    * keeps the loading quick and accurate.
    * @param string $class
    * @param string $path
    * @return null
    */
    public static function setClassPath($className, $path)
    {
        static::$classes[$className] = str_replace('\\' , '/' , $path);
    }

   /**
    * register an array of classname with paths to the file
    * keeps the loading quick and accurate.
    * @param array $classes
    * @return null
    */
    public static function setClassPaths(array $classes)
    {
        static::$classes = array_merge($classes, static::$classes);
    }

   /**
    * return the path to the classname registered.
    * @param string $class
    * @return string
    */
    public static function getClassPath($className)
    {
        return((isset(static::$classes[$className]) ?
            static::$classes[$className] : false));
    }

   /**
    * return the array of all the classes registered.
    * @return array
    */
    public static function getClassPaths()
    {
        return static::$classes;
    }

   /**
    * return a formated file path according to the PSR standard
    * @param string $class
    * @return string
    */
    public static function formatPSRClass($class)
    {
        return  str_replace(array('\\', '_'), '/', strtolower($class)) . '.php';
    }

   /**
    * attempt to load the registered class or interface.
    * @param string $class
    * @return bool
    */
    public static function loadClass($class)
    {
        $path = "";

        // check if the class has been mapped
        if(static::getClassPath($class) !== false)
        {
            // set the path as the mapped path
            $path = static::getClassPath($class);
        }

        // so no class mapped, lets try the PSR way
        else
        {
            // get the path from the class name according to PSR-0
            $path = str_replace('\\', '/', dirname(__FILE__)) . '/' . static::formatPSRClass($class);
        }

        // lets attempt to load the file and check if the class or interface exists
        if(file_exists($path))
        {
            @require $path;

            if(class_exists($class, false) || interface_exists($class , false))
            {
                return true;
            }
        }

        // throw an exception no class was loaded.
        else
        {
            throw new \Exception(sprintf('unable to load the requested class: %s at path %s', $class, $path));
        }
    }
}

I don't see a reason to use static methods here, consider using plain 'ol methods.
Why throw an exception? PHP will issue a fatal error when a class is used but not defined.
Since you are not using spl_autoload_register(), I'm not sure yours will play nicely with other autoloaders
Why not simply use the PSR-0 sample autoloader?


Encryption and decryption using alphabetic shifts My first piece of code encrypts text by moving each letter in the string 5 letters across in the alphabet.

encrypt = input('Enter text to encrypt : ')
encrypt = encrypt.lower().replace(" ", "")
for i in encrypt:
    print(chr(ord(i) + 5))
decrypt = input('Enter text to decrypt : ')
decrypt = decrypt.lower().replace(" ", "")
for i in decrypt:
    print(chr(ord(i) - 5))

The output of this code if I enter 'hello' would be 'mjqqt'.

To decrypt, I do the reverse, I move each letter 5 letters down the alphabet.

Below is my second piece of code that encrypts text against a code word. For example...

If I had 'abc' for the codeword and 'bcd' for the text to encrypt. Then: (a + b = 1 + 2) and this is repeated for each letter in the two words. The output of having my codeword as 'abc' and the word to encrypt and 'bcd' would be 'ceg'.

codeword = input('Enter codeword : ')
codeword = codeword.replace(" ", "")

encrypt = input('Enter text to encrypt : ')
encrypt = encrypt.replace(" ", "")

j = 0
for i in codeword:
    print(chr(ord(encrypt[j])+ ord(codeword[j])-96))
    j+=1

Part 1, Caesar cipher

Instead of printing one character at a time (inefficient), create a string using the str.join method.

''.join(chr(ord(char) + 5) for char in text)

The variable name i is associated with integer indexes and is misleading to use for a character.

Separate the encryption/decryption into a function

Rather than forcing printing text to screen, a function returning a string can be used in any other context as well. It also lets us use the similarity between encryption and decryption to reduce repetition in the code.

def caesar_shift(text, places=5):
    text = text.lower().replace(' ', '')
    return ''.join(chr(ord(char) + places) for char in text)

The number of places to shift is a parameter instead of hardcoded, with a default of 5 to work the same as your example when the argument is omitted. A value of -5 then decrypts the resulting string.

Handle out-of-range characters

The technique appears to be meant to only work on (English) letters. To avoid unintended results on different inputs, one option is to leave other characters unmodified.

Also, "wrap around" instead of substituting for characters outside of the alphabet, by using the modulus operator %.

from string import ascii_lowercase

def caesar_shift(text, places=5):
    def substitute(char):
        if char in ascii_lowercase:
            char_num = ord(char) - 97
            char = chr((char_num + places) % 26 + 97)
        return char
    text = text.lower().replace(' ', '')
    return ''.join(substitute(char) for char in text)

With the character substitution logic now being too long to fit comfortably in one line, I chose to separate it into a nested function.

Part 2, Vigenère cipher

Loop variables

Your loop here is somewhat confusing, in that the actual loop variable i goes unused while an additional variable j is used as the index. It could be written as follows to avoid that particular issue (using min in case the lengths are different).

for i in range(min(len(text), len(key))):
    print(chr(ord(text[i]) + ord(key[i]) - 96))

An even better, more idiomatic way to loop over multiple sequences (strings, lists etc.) is with the built-in zip function, which creates a tuple of elements for every loop iteration.

for text_char, key_char in zip(text, key):
    print(chr(ord(text_char) + ord(key_char) - 96))

Write as a function

Let's also apply the advice from the previous part and write it as a function.

def vigenere(text, key):
    return ''.join(caesar_shift(tc, ord(kc)-96) for tc, kc in zip(text, key))

Notice how it can be implemented using the caesar_shift function from above.

Handle different text and key lengths

What if the text to be encrypted or decrypted and the key are of different lengths? The convention is to repeat the key phrase.

While it wouldn't be too hard to write code for this from the beginning, it's often a good idea to look for something useful in the standard library. In this case, the itertools.cycle function turns out to be convenient.

from itertools import cycle

def vigenere(text, key):
    key = cycle(key)
    return ''.join(caesar_shift(tc, ord(kc)-96) for tc, kc in zip(text, key))

I've left out the input and output parts from this answer.


Which is the more preferable font size reset? Up until now, I've been using a 14-pixel reset in my theme's main CSS file, but after looking online, I see that a lot of web developers prefer a 10-pixel reset;

Here is what I'm currently using:

/* reset.css */
* { font-size: 1em; }

/* dark-theme.css */
body { font-size: 0.875em; } /* 14 pixels equals 1.0em */

Here is what I've seen preferred by others:

/* reset.css */
* { font-size: 1em; }

/* dark-theme.css */
body { font-size: 0.625em; } /* Now, 14 pixels equals 1.4em; easy! */

With a simple decimal calculation, I can see the latter method being easier to work with.  Other than that, is there any real upside to the 10px method, or any downside to my 14px method?

I use 14 pixels because I find that Tahoma and Segoe UI are both at optimum reading size for the majority of users, with text resize options available if required. A 16-pixel font size looks too cluttered for my personal taste.

The answer is neither.  The idea that 1em = 16px is only true in two scenarios:

The browser's default font-size is 16px and the user has not adjusted it
The user has specified that they want a default font-size of 16px because the browser's default was something other than 16px

There is no way to accurately convert between em and px. If you genuinely want a 14px font-size because your chosen font looks best at 14px, then specify that size:

html { /* or body, doesn't matter */
    font: 14px Tahoma, sans-serif;
}

You're still free to use relative font-sizes elsewhere if that's what you desire (I recommend that you do:  you will only have 1 location where you have to adjust your font-size).  Just keep in mind that your specified font may not be available on the user's device and the fallback font might have very different characteristics than you're expecting (eg. the letters might be the same height, but have a different width or vice versa).  You also might make your user upset because text is too small for them to read.

Honestly, the later example is a terrible practice that should have stopped a long time ago.  The idea that scaling everything to 10px makes math easier is a fallacy, especially when you consider that it is never safe to assume that 1em = 16px.

Here's a pretty common scenario showing why this important to understand:

Imagine your typical 2-column blog.  Sidebar on the left, blog content on the right.  How big do you make the sidebar?  You think to yourself, "300px sounds good", so you do the math:  300 / 16 = 18.75 and use 18.75em as your sidebar's width.  Now you want to add an image to your sidebar that's 300px wide.  Does it fit?  How well does it fit?

If the user's font-size is smaller (rare, but it could happen.  I've seen one source claim that iOS browsers have a default of 14px, but couldn't find anything confirming this), then the image overflows out of the sidebar.  If the user's font-size is larger because they have poor vision, then it fits with lots of extra space.  Extra space is usually ok, but you need to make sure you understand that this can happen and that you need to be able to plan for these scenarios.

Related:

http://filamentgroup.com/lab/how_we_learned_to_leave_body_font_size_alone/
http://csswizardry.com/2011/05/font-sizing-with-rem-could-be-avoided/


Python word guessing game def alphabetlist():
    print("alphabet1 = [a,b,c,d,e,f]")
    print("alphabet2 = [g,h,i,j,k,l]")
    print("alphabet3 = [m,n,o,p,q,r]")
    print("alphabet4 = [s,t,u,v,w,x]")
    print("alphabet5 = [y,z]")
print("Guess a word.")
print("Now i will try to figure out the word you guessed.")
totalletters=int(input("total number of letters in your guessed words are: "))
def chooselist(x):
    if x == 1:
        print(alphabet1)
    elif x == 2:
        print(alphabet2)
    elif x == 3:
        print(alphabet3)
    elif x == 4:
        print(alphabet4)
    elif x == 5:
        print(alphabet5)
list=[ ]
def listtranspose(w):
    if w == 1:
        i=int(input());list.append(alphabet1[i-1])
    elif w == 2:
        i=int(input());list.append(alphabet2[i-1])
    elif w == 3:
        i=int(input());list.append(alphabet3[i-1])
    elif w == 4:
        i=int(input());list.append(alphabet4[i-1])
    elif w == 5:
        i=int(input());list.append(alphabet5[i-1])
alphabetlist()
alphabet1 = ["a","b","c","d","e","f"]
alphabet2 = ["g","h","i","j","k","l"]
alphabet3 = ["m","n","o","p","q","r"]
alphabet4 = ["s","t","u","v","w","x"]
alphabet5 = ["y","z"]
step_1=[ ];h=0
while totalletters>h:
    print("your letter is from which list 1,2,3,4,5")
    x = int(input())
    step_1.append(x);h+=1
print("***\n  1    2    3    4    5    6   ")
g=0
while totalletters>g:
    chooselist(step_1[g]);g+=1
print("*******");e=0
while totalletters>e:
    print("choose from these lists again now 1,2,3,4,5,6")
    listtranspose(step_1[e]);e+=1
print("the word you guessed is: ",end=" ")
for i in range(0,len(list),1):
print(list[i],end="")

This program is a word guessing game. The user thinks of a word and this program guesses it. through a series of list I want to know if there are other ways to write this, more easily actually I get all the functions used but not the next part

I don't know what you mean by "more easily", but here are some comments on the code:
No error handling
In many statements like i=int(input());list.append(alphabet1[i-1]) you use user input directly to index a list, without checking if it is in range, or even if it is a number.
One wrong keystroke from the user and the whole program crashes.
Mixed code
You mix main script code with function definitions, making it harder to follow your program flow by hand, thus making your program harder to understand.
You should have your main program code follow all the functions in one coherent block, and it is even better to wrap it in its own main function, then use a construct like this:
if __name__ == '__main__':
    main()

Repetitive code
In both listtranspose and chooselist you write the same line 5 times over, just because you used separate variables for 5 (almost) identical lists instead of a list of lists.
Naming
Do not use list as a variable name! It is a Python builtin for creating new lists, and while Python is nice enough to allow you to overwrite it, this can both break future code and make your code harder to understand by others.
There are seemingly random one letter variables in your code h, g, e, it is hard to understand what they do.
Please give them more meaningful names.
Function alphabetlist actually prints the lists, so it would be better to name it print_lists or similar.
Also, there is no reason to hardcode the lists you print, since they already exists as list type variables.


Fine tuning of an informative replacement for rm I want to replace rm by a more informative variant: I would like to see which files will be deleted, along with their size, and I would like this information to use the same coloring as ls. As an example (please imagine the coloring):

# remove -r foo/ bar/*
4K   bar/file1
1.2M foo/
6.1M bar/file2
remove -r 3 files, 7.3M [yn]? _

I built on suggestions found in another question, and now have a bash script that I begin to like. I have been using it for a few days without noticing any obvious errors. However, I would appreciate any help with making sure that the script will not misbehave in cases I did not foresee.

Here is my code for the coloring:

du_colored (){
    # read ls --color output into ls_colored_array
    # use \13 as a trick to handle names with spaces
    read -d '\n' -r -a ls_colored_array <<< $(ls -Ad --color "$@" | tr " " "\13")

    if [[ ${#ls_colored_array[@]} = 0 ]]; then return 1; fi

    # - loop over the array and issue du -sh for every element (without coloring)
    # - exchange du's ouput with ls's
    # - finally sort the output
    for i in "${ls_colored_array[@]}"; do
        i=`echo $i | tr "\13" " "`
        printf '%s' "${i}" | sed -r "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g" | \
            xargs -n1 -0 du -sh | awk -v i="$i" '{ printf "%-4s ", $1; print i }'
    done | sort -h
}

and the removal script:

# Separate options to rm from 'real' arguments
for i in "$@"; do
    case $i in
        -*) options+=( "$i" ) ;;
        *)  toRemove+=( "$i" ) ;;
    esac
done

# Print file list
# Abort if no files match
du_colored "${toRemove[@]}"
if [ "_$?" != "_0" ]; then exit 1; fi

# Print summary
count=$(find "${toRemove[@]}" 2>/dev/null | wc -l 2>/dev/null)
size=$(du -sch "${toRemove[@]}" 2>/dev/null | tail -1 | tr '\t' ' ')
plural="s"
if [ $count -eq 1 ]; then plural=""; fi
printf "remove ${options[@]} ($count file$plural, $size) [ny]? " ; read
if [ "_$REPLY" = "_y" ]; then
    /bin/rm ${options[@]} ${toRemove[@]}
else
    echo '(cancelled)'
fi

Does this look safe in the sense that what du_colored presents is exactly what will be deleted? Both files and directories seem to be treated correctly, even if they include spaces or wildcards. Are there scenarios that I missed? An obvious one are file names that contain a \13 character.
The initial ls in du_colored throws errors for non-existing files, which I like because it mimicks the behavior of regular rm. However, I currently discard all error messages of the final find and grep commands, such that non-existing files do not produce multiple errors. This seems like a dirty trick. Is there a cleaner way?
Bonus question: Is there an easy way to make the output use multiple columns, just as a regular ls would? I tried several ways to pipe the output of du_colored to other tools, but either the colors were lost completely or the control characters for the coloring broke the alignment, as they do in ls --color | pr -2 -t.

Bonus question: Does ls -C help?

-C     list entries by columns

You need -r to remove directories, so do you intend to impose this check as well when directories are specified?

plural="s"
if [ $count -eq 1 ]; then plural=""; fi

That can be rewritten as

[ $count -eq 1 ] && plural="" || plural="s"

Is the use of _ really necessary here?

if [ "_$?" != "_0" ]; then exit 1; fi
if [ "_$REPLY" = "_y" ]; then

edit: Bonus question, round 2...

You could try this for du_colored:

du_colored() {
    du -sh "$@" | awk -F'\t' -v COL=4 \
        '{if(++c>COL){c=1;print""}("ls -Ad --color \""$2"\"")|getline entry;
        printf "%6s %s\t",$1,entry}END{print""}' | \
    column -s $'\t' -t
}

Instead of parsing ls output carefully to feed into du, I started from the output of du and then replacing each entry with its ls --Ad --color output. COL is used to fix the number of 'columns' we want column to pick up afterwards, which is in turn used to print the appropriate newlines.

Finally, I feed the output to column as a final attempt at pretty-printing. \t is used as the delimiter throughout.

It still doesn't look as nice as the default non-piped ls output, but at least it's something I suppose...

edit 2: Bonus question, round 3...

I hope this is what's required... :)

du_colored() {
    awk -F'/' 'NR==FNR{k=$0;gsub(/\x1B\[[;[:digit:]]*m/,"",k);s[k]=$0}
        NR!=FNR{l=$0;for(i=3;i<=NF;i+=2){v=$i;gsub(/[ ]+$/,"",v);sub(v,s[v],l)};
            gsub("/"," ",l);print l}' \
    <(ls -Ad --color "$@") \
    <(du -sh "$@" | awk -F'\t' -v C=4 '{if(++c>C){c=1;print""}printf "/%6s/%s\t",$1,$2}
        END{print""}' | column -s $'\t' -t)
}

Get both the output of ls -Ad --color "$*" and du -sh "$@" ... using bash process substitution to pass into awk.

The du -sh command is in turn piped to an 'internal' awk command, similar to my earlier suggestion, for column to render properly. What's crucial to note here is that the start, the size and the file/directory entry are delimited using the / character, which is the safest delimiter as files and directories cannot have that (nested directories yes though).

The 'main' awk command uses / to split each line into 1 + C * 2 columns (C=4 used above). By using the combination of NR==FNR/NR!=FNR conditions, we can specify different handling for the two 'files'.

For the ls output, i.e. NR==FNR, we construct a map from the file/directory (by stripping away the color codes - gsub(/\x1B\[[;[:digit:]]*m/,"",k)) to its colored output.
For the du output, we then substitute the file/directory in the appropriate fields (after trimming the extra spaces behind - gsub(/[ ]+$/,"",v)) with the colored output from the mapping.
After substituting each / character back to a  character, we reprint the entire line, which now contains the colored output.

This is arguably more efficient as both ls and du are performed once on the arguments. The lesson learned here is that column doesn't play ball with most control characters (seemingly \t is the only exception here). For some reason, sort-ing on du's output is still introducing minor quirks in my own testing, so I'm leaving that out for now.


Prepending textbox to Wikipedia thumbnail picture I'm working on small Javascript code that adds text from a clicked-href to the top right corner of the Wikipedia page. So for example, if I click the "Jamaican" link in Sly and Robbie, a textbox would be appended to the page on the top right.

Note: to test this code, copy it and paste it to your console, then click some links.

/**
 * My Name
 * November 24th 2015
 * Device: Mac OS X Yosemite 10.10.5
 * Editor - vim
 * Browser - Google Chrome Version 46.0.2490.86 (64-bit)
 */

'use strict';

// Detect click on href
$("a").click(function (event) {
    // Capture the href from the selected link
    var link = this.href;
    $.ajax({
        type: 'get',
        url: link
    }).done(function (data) {

        isValidWikiPage(link);

        // Find only the body text (not the titles.. or unnecessary text tips)
        var bodyText = $(data).find('#bodyContent #mw-content-text p').text();

        // Prepend the text only if there is text in the clicked link
        if (bodyText.length > 0) {
            prependText(bodyText);
        } else {
            alert("No text found!");
        }
    });

    // Prevent the link from being executed
    return false;
});

/**
 * Checks the URL to see if the link clicked
 * is a valid Wikipedia page
 * @param  {[type]}  link The clicked URL
 */
function isValidWikiPage(link) {
    // Check if link clicked is a Wikipedia page
    if (link.indexOf('wikipedia.org') <= 0) {
        // Show an alert
        alert(link + " is not a Wikipedia page!");
        // Redirect user to the new page
        window.location.href = link;
        return;
    }
}

/**
 * Prepend the thumbnail with clicked body text
 * @param  {string} text Body text
 */
function prependText(text) {
    $(".infobox .vcard .plainlist").addClass($(".infobox tbody").prepend(text));
}

Nice idea.
Here's a solution that breaks down each of the steps into tiny, single purpose functions, whose names reveal exactly what they do.  This removes all the nesting and makes the solution easier to follow.
New logical steps:

Identifying a valid link (this was always broken out in the oringal solution)
"Upgrading" a link into a preview link (this is no longer coupled to 1., and can be re-used for other links)
Fetching the link's html
Extracting just the body text from the html
Filling the preview window with text

Some ideas that might be nice:

It might be nice to have the preview window vanish if you click the same link again.
Remove old contents from preview window, rather than prepending

Other notes:

I put the each statement that kicks everything off at the bottom, so that you can still copy paste into the console, even though it's more logical to have it at the top, since it's the "main" method, so to speak.
I removed the alerts, since I didn't think they were necessary.

Refactored code:
function turnIntoPreviewLink(link) {
  $(link).click(function(e) { e.preventDefault(), previewContents(link.href); })
}

function previewContents(wikiUrl) {
  grabHtml(wikiUrl).done(function(html) {
    addContentToPreview(bodyText(html));
  })
}

function grabHtml(wikiUrl) {
  return $.ajax({ type: 'get', url: wikiUrl });
}

function bodyText(html) {
  return $(html).find('#bodyContent #mw-content-text p').text();
}

function addContentToPreview(text) {
    $(".infobox .vcard .plainlist").addClass($(".infobox tbody").prepend(text));
}

function isValidWikiPage(url) {
  return url.indexOf('wikipedia.org') >= 0;
}

$("a").each(function(i, link) {
  if (isValidWikiPage(link.href)) turnIntoPreviewLink(link);
});


Checking and converting [[HH:]MM:]SS input format I would like to use the following routine in my job submission bash script which expects the allowed walltime in the format [[HH:]MM:]SS. (Brackets indicating optionality.) For easier readability I want to convert any input value into the fixed format HH:MM:SS where the value for hours is open ended.
I think it works okay, but I am a bit worried if I have overlooked possible points where a user could make a mistake. This could lead to submitting an invalid jobscript and a lot of computation jobs not starting.
I also ask myself whether or not my code is optimal.

#!/bin/bash

#
# If there is an unrecoverable error: display a message and exit.
#

printExit ()
{
    case $1 in
        [iI]) echo INFO: "$2" ;;
        [wW]) echo WARNING: "$2" ;;
        [eE]) echo ERROR: "$2" ; exit 1 ;;
           *) echo "$1" ;;
    esac
}

#
# Test if a given value is an integer
#

isnumeric ()
{
    if ! expr "$1" : '[[:digit:]]*$' > /dev/null; then
        printExit E "Value for $2 ($1) is no integer."
    fi
}

#
# Test if a given value is a proper duration
#

istime ()
{
    local tempTime=$1
    # Split time in HH:MM:SS
    theTempSeconds=${tempTime##*:}
    isnumeric "$theTempSeconds" "seconds"
    if [[ ! "$tempTime" == "${tempTime%:*}" ]]; then
      tempTime="${tempTime%:*}"
      theTempMinutes=${tempTime##*:}
      isnumeric "$theTempMinutes" "minutes"
    fi
    if [[ ! "$tempTime" == "${tempTime%:*}" ]]; then
      tempTime="${tempTime%:*}"
      theTempHours=${tempTime##*:}
      isnumeric "$theTempHours" "hours"
    fi
    if [[ ! "$tempTime" == "${tempTime%:*}" ]]; then
      printExit E "Unrecognised format."
    fi

    theSeconds=`expr $theTempSeconds % 60`
    theTempMinutes=`expr $theTempMinutes + $theTempSeconds / 60`
    theMinutes=`expr $theTempMinutes % 60`
    theHours=`expr $theTempHours + $theTempMinutes / 60`

    printf -v theWalltime "%d:%02d:%02d" $theHours $theMinutes $theSeconds
}

theWalltime="$1"

echo "$theWalltime"

istime $theWalltime

echo "$theWalltime"

exit 0

I use the first two functions quite a lot in my script so I used them for the target function also.

Here are my suggestions:

The printExit() prints an error message, but does not exit.  It would be better to actually exit or rename the function.
Your regex in isnumeric() has two minor issues.  Putting a ^ at the beginning will make it clear that the regex matches the whole string.  While this is implicit with expr it does not fit with how most implementations work.  Another issue is that the * quantifier means 0 or more so it might match 0 or none of the what you're looking for.  So an empty string would match what you have since it would be zero digits followed by the end of the string.  I couldn't reproduce this failure mode, but I'd recommend this line instead which works for my tests:

if ! expr "$1" : '^[[:digit:]]\+$' > /dev/null; then

It would be nice to explain what you're trying to do with those string manipulations with a few comments
More of your variables could be localized so they don't leak out of the function they are used in.
Using the in variable names to avoid conflicting with generic names works, but if you can find more descriptive names it would be easier to follow.  Along the same lines all variables are temporary so that doesn't help explain why the variable exists.  Maybe checkTime or even check_time.
All of the math is integer math.  You won't get any decimals with bash or expr math.  https://stackoverflow.com/questions/12722095/how-can-i-get-float-division explains that you can work around with bc.
Optimization Using expr is fine, but it calls an external program.  Using bash's builtin math would avoid spawning a new process and would cut out a lot of the script's time.
    ((just_minutes = just_minutes + trunc_seconds/60))


Pythonic style guide Which of these 3 python snippets is more pythonic?

This 1 liner list comprehensions which is a little overcomplex

users_to_sent = [(my_database.get_user_by_id(x), group['num_sent'][x]) for x in user_ids]

or this multi liner which is 'too many lines of code'

users_to_sent = []
for id in user_ids:
    t1 = my_database.get_user_by_id(id)
    t2 = group['num_sent'][id]
    users_to_sent.append( (t1,t2) )

or should the 1 liner be spun out into a separate function and called from a list comprehension?

def build_tuple(x, my_database, group):
    return (my_database.get_user_by_id(x), group['num_sent'][x])

users_to_sent = [build_tuple(x, my_database, group) for x in user_ids]

You'd definitely need to name your function something more descriptive than build_tuple for it to be a good idea (and same with t1 and t2 in the multi-liner!).

I'd use a function if it's something you do more than once or twice, otherwise I'd probably stick with the list comprehension - I find it easier to read than the multi-liner version.

If I was doing a function, I'd probably make it generate the whole list - is there any reason not to do that?

def make_users_to_sent(user_ids, my_database, group):
    return [(my_database.get_user_by_id(x), group['num_sent'][x]) for x in user_ids]

users_to_sent = make_users_to_sent(user_ids, my_database, group)


Pomodoro timer in Go My goal is to implement a simple pomodoro timer using Go: channels, goroutines.
I'm a newbie in Go world and have some misunderstanding about naming convention. I read a lot of Docker's code on GitHub and there I saw that people sometimes use very short names. These names are not clear for me (a man who comes from C++ world).
func main() {
   app := pomodoro.NewPomodoro()
   app.Start()
}

Give me a review about structure's name
type (
    PomodoroManager struct {
        dur    Duration
        interv Interval
        ch     Chanel
        serv   PomodoroService
    }

    Duration struct {
         working    int
         shortBreak int
         longBreak  int
    }

    Interval struct {
        shortInterval int
        longInterval  int
    }

    Chanel struct {
        start         chan bool
        endShortBreak chan bool
        endLongBreak  chan bool
        end           chan bool
    }
)

func NewPomodoro() *PomodoroManager {
    dur := Duration{
        working:    25,
        shortBreak: 5,
        longBreak:  15,
    }

    interv := Interval{
        shortInterval: 1,
        longInterval:  5,
    }

    ch := Chanel{
        start:         make(chan bool),
        endShortBreak: make(chan bool),
        endLongBreak:  make(chan bool),
        end:           make(chan bool),
    }

    serv := PomodoroService{}

    return &PomodoroManager{dur, interv, ch, serv}
}

func (p *PomodoroManager) Start() {
    go p.serv.StartWorking(p.ch.start, p.dur.working)
    go p.startServManager(p.ch.start, p.ch.endShortBreak, p.ch.endLongBreak, p.ch.end)

    <-p.ch.end
}

func (p *PomodoroManager) startServManager(start, endBreak, endLong, end chan bool) {
    for {
        select {
        case afterWorking := <-start:
            _ = afterWorking
            if p.interv.shortInterval == p.interv.longInterval {
                go p.serv.StartLongBreak(endLong, p.dur.longBreak)
            } else {
                p.interv.shortInterval += 1
                go p.serv.StartBreak(endBreak, p.dur.shortBreak)
            }
        case endShortRelax := <-endBreak:
            _ = endShortRelax
            go p.serv.StartWorking(start, p.dur.working)

        case endLongRelax := <-endLong:
            _ = endLongRelax

            userAnswer := askUser()
            if userAnswer == "Y" {
                p.interv.shortInterval = 1
                go p.serv.StartWorking(start, p.dur.working)
            } else {
                end <- true
            }
        }
    }
}

func askUser() string {
    fmt.Println("Do you want to coninue? Y/N")

    var answer string
    fmt.Scanln(&answer)

    return answer
}

And Last component of the program.
import (
    "os/exec"
    "time"
)

type (
    Service interface {
        StartWorking(chan bool, int)
        FinishBreak(chan bool, int)
        FinishLongBreak(chan bool, int)
    }

    PomodoroService struct {
    }
)

func (p *PomodoroService) StartWorking(start chan bool, duration int) {
    exec.Command("say", "Start working").Output()
    time.Sleep(time.Millisecond * time.Duration(duration))
    exec.Command("say", "Time to Relax").Output()
    start <- true
}

func (p *PomodoroService) StartBreak(endBreak chan bool, duration int) {
    exec.Command("say", "Start short relax").Output()
    time.Sleep(time.Millisecond * time.Duration(duration))
    exec.Command("say", "Relax has finished, getting back to job").Output()
    endBreak <- true
}

func (p *PomodoroService) StartLongBreak(endLong chan bool, duration int) {
    exec.Command("say", "Start long relax").Output()
    time.Sleep(time.Millisecond * time.Duration(duration))
    exec.Command("say", "Long pause has finished").Output()
    endLong <- true
}

Your NewPomodoro function should simply be called New (the fact that you're creating a Pomodoro thing is inherent in "you're calling pomodoro.New" and "there's only one New", if there were more, it would make sense to have, say, NewTimer and NewManager).
I don't see what the PomodoroService is actually useful for, it seems to be abstraction for the sake of abstraction. I would simply put the StartBreak, StartLongBreak, and StartWorking methods directly on the timer/manager, as long as this is essentially all there is.
I would probably have called PomodoroManager something else. Probably Pomodoro, PomodoroTimer or (most probably) just Timer (the fact that it's a Pomodoro timer is inherent from being in the pomodoro package).
I am ambivalent about having the various timers and channels in sub-structs. There's few enough that I am not 100% sure not doing so makes the code mode readable. However, since the members are not exported, their types do not need to be exported, since they can't be accessed from outside the package anyway. I would also name the structs in plural, since they clearly contain "channelS", "durationS" and "intervalS".
Since you're never seemingly using the values passed across your channels, you might as well make then chan struct{} instead of chan bool.
I would consider making your intervals to be of type time.Duration instead of int.
I would consider wrapping all the exec.Command("say", ...).Output()" calls in a non-exported utility function (called, say, say).


Contiguous elements of the list sum to K The task:

  Given a list of integers and a number K, return which contiguous
  elements of the list sum to K.

  For example, if the list is [1, 2, 3, 4, 5] and K is 9, then it should
  return [2, 3, 4].

My solution

// init
const arr = [1, 2, 3, 4, 5];
const K = 9;

// main
const getContiguousElementsSum = (arr, K) => {
  if (arr.length === 0 || K < 0) {
    return null;
  }
  let sum = 0;
  const returnResult = [];
  const found = arr.some(x => {
    sum += x;
    returnResult.push(x);
    return sum === K;
  });
  // exit case
  if (found) {
    return returnResult;
  }
  // recursive case
  return getContiguousElementsSum(arr.slice(1), K);
};
console.log(getContiguousElementsSum(arr, K));

Is there a shorter and/or more readable solution?

// init
const arr = [1, 2, 3, 4, 5];
const K = 9;

// main
const getContiguousElementsSum = (arr, K) => {

It seems to me to be unnecessarily confusing to alias variable identifiers like this.

  if (arr.length === 0 || K < 0) {
    return null;
  }

I don't see this anywhere in the specification you've quoted. I would expect the function to handle K === 0 by returning [], and to give [-1,-2,-3] for getContiguousElementsSum([-1,-2,-3,-4], -6).

Also, since the specification doesn't say anything about error cases, I would assume by default that it should throw something (probably RangeError) if no matching subarray can be found.

  const found = arr.some(x => {
    sum += x;
    returnResult.push(x);
    return sum === K;
  });

It would be kinder to the garbage collector to just track indexes and use slice when you find a match.

  // recursive case
  return getContiguousElementsSum(arr.slice(1), K);

The overall complexity of this implementation is \$O(n^2)\$. It can be done quite easily in \$O(n \lg n)\$ in full generality using just arrays, and in \$O(n)\$ if you have an efficient set implementation, by using the property $$\sum_{i=a}^b A[i] = \left(\sum_{i=0}^a A[i]\right) - \left(\sum_{i=0}^{b-1} A[i]\right)$$To build an array of \$\left(\sum_{i=0}^a A[i]\right)\$ takes linear time; then either convert it into a set or sort it; and for each element in the array of partial sums you need to test for the presence of that partial sum minus the target.

And, of course, it can be done in \$O(n)\$ time with just arrays if you're guaranteed that the elements are positive, as described in janos' answer.


Using a public API to display data It's a simple app that uses a public API to display data. It works as is, but I'm seeing a lot of spaghetti code that my limited experience can't fix on its own.

import React, { Component } from "react";
import Spinner from "./components/common/Spinner";
import "../src/App.css";

class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      items: [],
      isLoading: false
    };
  }

  componentDidMount() {
    const urlParams = new URLSearchParams(window.location.search);
    const city = urlParams.get("city");
    fetch(`http://opentable.herokuapp.com/api/restaurants?city=${city}`)
      .then(res => res.json())
      .then(json => {
        this.setState({
          isLoading: true,
          items: json
        });
      });
  }
  render() {
    const { isLoading, items } = this.state;
    let content =
No Data
;
    if (!items.restaurants || items.restaurants === []) {
      content =
No Data
;
    } else if (items.restaurants.length !== 0) {
      console.log(items);
      let itemsToArray = Object.values(items.restaurants);
      content = (

              {itemsToArray.map(item => (

                    {item.name}
                  {" "}
                  | {item.address}{" "}
                  | Price rating:{" "}
                  {item.price}

              ))}

      );
    }
    return !isLoading ? (

        <Spinner />

    ) : (
      <React.Fragment>

{content}

      </React.Fragment>
    );
  }
}

export default App;

I can think of mutliple ways to improve this code.

First, error handling. In your case, not receiving data from your API call may cause your program to crash.

You will have to add an error variable in your state :

this.state = {
    items: [],
    isLoading: true,
    error: ''
};

And add a .catch at the end of your fetch function (or use the new async/await syntax surrounded by a try/catch block).

You should also put your response formatting here instead of the rendering function, to avoid reformatting your data everytime your component is rerendered :

fetch(`http://opentable.herokuapp.com/api/restaurants?city=${city}`)
    .then(res => res.json())
    .then(json => {
        this.setState({
            isLoading: false,
            items: Object.values(json.restaurants) //Data formatting
        });
    })
    .catch(error =>{
        this.setState({
            isLoading: false,
            error // Sends the error object (or message, I am not sure about the exact syntax)
        });
    });

I also noticed that you set your isLoading state to true when you finished loading and to false in your initial state so I reversed it.

Now, the rendering.

JSX allows you to put conditions inside your HTML, making the syntax of your render way more readable and avoid code duplication :

render() {
    const { isLoading, items, error } = this.state;

    return (

                            {isLoading && <Spinner />}
                            {error &&
Error : {error}
}
                            {!isLoading && !items.length &&
No Data
}

 // Did you forget this node ?
                                {items.map(({ id, reserve_url, address, price, name }) => //Won't do anything if the items array is empty

                                            {name}
                                        {" "}
                                        | {address}{" "}
                                        | Price rating:{" "}
                                        {price}

                                )}

    )
}

I also deconstructed your parameters in the map function.

Working example :

class App extends React.Component {
    constructor(props) {
        super(props);
        this.state = {
            items: [],
            isLoading: true,
            error: ''
        };
    }

    componentDidMount() {
        const urlParams = new URLSearchParams(window.location.search);
        const city = urlParams.get("city");

        fetch(`http://opentable.herokuapp.com/api/restaurants?city=London`) // Hard coded London
            .then(res => res.json())
            .then(json => {
                setTimeout(() => { //Just to see the loading element / spinner
                    this.setState({
                        isLoading: false,
                        items: Object.values(json.restaurants)
                    });
                }, 1500)
            })
            .catch(error =>{
                this.setState({
                    isLoading: false,
                    error
                });
            });
    }

    render() {
        const { isLoading, items, error } = this.state;

        return (

                                {isLoading &&
Loading...
}
                                {error &&
Error : {error}
}
                                {!isLoading && !items.length &&
No Data
}

                                    {items.map(({ id, reserve_url, address, price, name }) =>

                                                {name}
                                            {" "}
                                            | {address}{" "}
                                            | Price rating:{" "}
                                            {price}

                                    )}

        )
    }
}

ReactDOM.render(<App/>, document.getElementById('root'))


Editing e-commerce website values Each product in my e-commerce website has Arabic and English values for the title, description and excerpt. I have this method EditProduct to update those values based on current culture (Arabic or English)

public void EditProduct(string element, string text1, string text2, bool edit1, bool edit2, string culture)
{
    var product = DoSomeMagicToGetAProduct();

    if (element == "title")
    {
        if (culture == "en")
        {
            if (edit1)
            {
                product.Title = text1;
            }
            if (edit2)
            {
                product.TitleAr = text2;
            }
        }

        if (culture == "ar")
        {
            if (edit1)
            {
                product.TitleAr = text1;
            }
            if (edit2)
            {
                product.Title = text2;
            }
        }
    }

    if (element == "description")
    {
        // similar codes here
    }

    if (element == "excerpt")
    {
        // similar codes here
    }

    product.Save();

}

The method works well, but I think I can improve the way I write it to be more elegant.
Any suggestions?

I think the code is a little dodgy, and the design is dodgy. I'd store the translation in a dictionary of some sort.

But addressing your immediate question, to simplify the code you have, you could do....

public void EditProduct(string element, string text1, string text2, bool edit1, bool edit2, string culture)
        {
            var product = DoSomeMagicToGetAProduct();

            var arText = culture == "ar" ? text1 : text2;
            var enText = culture == "en" ? text1 : text2;
            var updateEn = culture == "en" ? edit1 : edit2;
            var updateAr = culture == "ar" ? edit1 : edit2;

            if (element == "title")
            {
                if (updateEn) product.Title = enText;
                if (updateAr) product.TitleAr = arText;
            }

            if (element == "description")
            {
                if (updateEn) product.Description = enText;
                if (updateAr) product.DescriptionAr = arText;
            }

            if (element == "excerpt")
            {
                if (updateEn) product.Excerpt = enText;
                if (updateAr) product.ExcerptAr = arText;
            }

            product.Save();

        }


node.js connecting to s3 and youtube I have a task to connect to S3 get a JSON file with the names of artists, use the youtube-data-api and search for the top 10 songs.
Then send it back in a given format to S3 back.

console.log("Loading up the best code ever!!!");

//packagess import
var fs = require('fs');
var AWS = require('aws-sdk');
var Singer = require('./Singer')

//prepare all the AWS S3 data
AWS.config.update({ region: "us-west-1" });
var credentials = new AWS.SharedIniFileCredentials();
AWS.config.credentials = credentials;
// Create S3 service object
s3 = new AWS.S3({ apiVersion: '2006-03-01' });

// Create the parameters for calling createBucket
var bucketParams = {
    Bucket: 'pc-backend-exercises',
    Key: 'toSearch.json',
    ResponseContentType: 'application/json'
};

/// prepare the youtube-data-api
var YTAPI = require('node-youtubeapi-simplifier');
var APIKEY = 'AIzaSyDS1p3m9wnZLLCPc1hDQBX3K_UnS4j0CdY'
YTAPI.setup(APIKEY);

console.log('connecting to s3 to get the json file');

var singers = [];

s3.getObject(bucketParams, function (err, data) {
    // Handle any error and exit
    if (err) {
        console.log(err, err.stack);
        return err;
    }
    var fileContents = data.Body.toString();
    var json = JSON.parse(fileContents);

    for (var i = 0; i < json.Search.artists.length; i++) {
        var newSinger = new Singer(json.Search.artists[i]);
        singers.push(newSinger);
    }
    console.log('the list of the singers has %d singers',json.Search.artists.length);
    search10TopForASinger();
});

//this function searches using youtube-api-simplfier for each artist name
//returns the list of songs and collects only 10, stores them into a map in the singer object
function search10TopForASinger() {
    console.log('entered search10TopForASinger ')
    var promises = [];

    for (var i = 0; i < singers.length; i++) {
        //Gets only 10 results
        promises.push(YTAPI.searchFunctions.simpleSearch(singers[i].name));
    }
    Promise.all(promises)
           .then((results) => {
               console.log('all the threads of the youtube api are back')
               for (var i = 0; i < singers.length; i++) {
                   setSongsArray(results[i], singers[i].songs);
               }

               printJsonAndUpload();
               console.log('we are done you can test yourself with testGiladFile.js');
            })
             .catch((e) => {
                    // Handle errors here
            });

}

//sets 10 or less songs for each artist into an array
//we use the Singer object in order to store the data
function setSongsArray(data, songs) {
    console.log('entered setSongsArray')
    var size = 10;
    if (data.length < 10) {
        size = data.length;
    }

    for (var i = 0; i < size; i++) {
       songs.push(data[i].title);
    }
}

//The team wanted a json file with a certain format
//we create here to format and upload it back to S3
function printJsonAndUpload() {

    console.log('entered printJson')
    var data = {
        results: []
    };

    for (var i = 0; i < singers.length; i++) {
        var singerName = singers[i].name;
        var singerObj = {}
        singerObj[singerName] = []
        for (var song = 0; song < singers[i].songs.length; song++) {
            var songObj = {};
            songObj[song+1] = singers[i].songs[song];
            singerObj[singerName].push(songObj)
        }
        data.results.push(singerObj);
    }

    console.log('uploading file to s3')
    s3.putObject({Bucket: 'pc-backend-exercises',Key:'gilad.json',Body: JSON.stringify(data), ContentType: "application/json"},
        function(err, data) {
            //console.log('error');
            //console.log(JSON.stringify(err)+" "+JSON.stringify(data));
        });
    console.log('the name of the file is gilad.json');
}

This is the first code I have ever written in Node.js, please give me any feedback about style and performance.

I had a crack at it, I tried to replace the for loops with higher-order functions, and use ES6 let and const for variable declarations.  I feel like the efficiency of the functions could be improved, but this could be a good starting point to refactoring.  Also, fixed some formatting to make it a bit easier to read.

const AWS = require('aws-sdk');
const fs = require('fs');
const Singer = require('./Singer')
const YTAPI = require('node-youtubeapi-simplifier');

const credentials = new AWS.SharedIniFileCredentials();
const s3 = new AWS.S3({ apiVersion: '2006-03-01' }); //does s3 need to be declared as variable?

const APIKEY = 'AIzaSyDS1p3m9wnZLLCPc1hDQBX3K_UnS4j0CdY'

const bucketParams = {
  Bucket: 'pc-backend-exercises',
  Key: 'toSearch.json',
  ResponseContentType: 'application/json'
};

AWS.config.update({ region: "us-west-1" });
AWS.config.credentials = credentials;
YTAPI.setup(APIKEY);

const singers = []; //do you need this singers array?

s3.getObject(bucketParams, (err, data) => {
    if (err) {
      console.log(err, err.stack);
      return err;
    }

    let fileContents = data.Body.toString();
    let json = JSON.parse(fileContents);
    //I try to avoid for loops if possible
    json.Search.artists.forEach((artist, idx) => singers.push(new Singer(json.Search.artists[idx])));

    console.log('the list of the singers has %d singers',json.Search.artists.length);
    search10TopForASinger();
});

const setSongsArray = (data, songs) => {
  data.slice(0, 10).forEach((song) => songs.push(song.title));
}

const search10TopForASinger = () => {
  console.log('entered search10TopForASinger ')

  let promises = singers.map((singer, idx) => YTAPI.searchFunctions.simpleSearch(singers[idx].name));

  Promise.all(promises).then((results) => {
    console.log('all the threads of the youtube api are back')
    singers.forEach((singer, idx) => setSongsArray(results[idx], singers[idx].songs) )
    printJsonAndUpload();
    console.log('we are done you can test yourself with testGiladFile.js');
  }).catch((e) => {
    console.log(e)
  });
}

const printJsonAndUpload = () => {
  console.log('entered printJson')
  const data = {
    results: []
  };

  singers.forEach((singer, idx) => {
    let singerName = singers[idx].name;
    const singerObj = {};
    singerObj[singerName] = [];
    singers[idx].songs.forEach((song) => { //could use .reduce() here
      const songObj = {};
      songObj[song+1] = singers[i].songs[song];
      singerObj[singerName].push(songObj)
    });
    data.results.push(singerObj);
  });

  console.log('uploading file to s3')
  s3.putObject({ Bucket: 'pc-backend-exercises', Key:'gilad.json', Body: JSON.stringify(data), ContentType: "application/json" },
    (err, data) => {
      //console.log('error');
      //console.log(JSON.stringify(err)+" "+JSON.stringify(data));
    });
  console.log('the name of the file is gilad.json');
}


Is there a better way to assign two variable to two others conditionally? I wrote this code, but it feels like it could be a bit more elegant:

Relevant code only:

GameObject newBall;
GameObject pathBall;
if (tag.Equals(pathBallTagName))
{
    newBall = other.gameObject;
    pathBall = gameObject;
}
else
{
    newBall = gameObject;
    pathBall = other.gameObject;
}

Entire .cs file:

using UnityEngine;
using System.Collections;
using BallDelegate;

public class BallCollider : MonoBehaviour {

    public Spline mySpline;
    private SplineController mySplineController;

    public AllBallManager abm;

    public string pathBallTagName = "PathBalls";
    public string ballTagEnd = "Balls";

    public BoxCollider front;
    public BoxCollider back;
    public BoxCollider spawnArea;

    public string frontName = "BallBox Front";
    public string backName = "BallBox Back";

    public bool isInSpawn = true;

    void Start() {
        mySplineController = GetComponent<SplineController> ();
        BoxCollider[] frontBack = GetComponentsInChildren<BoxCollider> ();
        foreach (BoxCollider bc in frontBack)
            if (bc != null) {
                if (bc.name.Equals (frontName))
                    front = bc;
                else if (bc != null && bc.name.Equals (backName))
                    back = bc;
            }

    }

    void OnTriggerEnter(Collider other) {
        if (mySplineController)
        {
            mySplineController.go = true;
            if (mySpline)
                mySplineController.gravSpline = mySplineController.currentSpline = mySpline;
        }
        if (other == spawnArea) {
            isInSpawn = true;
        }
        else if (!isInSpawn) {
            BallDelegateCS bd = other.gameObject.GetComponent<BallDelegateCS>();
            if (bd) // only balls, no other game objects
            {
                GameObject newBall;
                GameObject pathBall;
                if (tag.Equals(pathBallTagName))
                {
                    newBall = other.gameObject;
                    pathBall = gameObject;
                }
                else
                {
                    newBall = gameObject;
                    pathBall = other.gameObject;
                }
                while (newBall.transform.parent)
                    newBall = newBall.transform.parent.gameObject; // make sure we don't get BallBall
                abm.MakeWayFor(newBall, pathBall);
            }
        }
    }

    void OnTriggerExit(Collider other) {
        if (other == spawnArea) {
            isInSpawn = false;
        }

    }
}

One alternative, note how we must store the bool result to ensure identical semantics (otherwise you risk calling tag.Equals twice... which in addition to performance considerations might change its result between calls or have side effects!)

bool swap = tag.Equals(pathBallTagName);
GameObject newBall = swap ? other.gameObject : gameObject;
GameObject pathBall = swap ? gameObject : other.gameObject;

Alternatively one could implement a Swap function elsewhere:

GameObject newBall = other.gameObject;
GameObject pathBall = gameObject;
if (tag.Equals(pathBallTagName)) Swap (ref newBall, ref pathBall);


Perm-Missing-Elem: 100% functional score, but only 60% performance I cannot figure out why I didn't get 100% success from the Codility's Perm-Missing-Element test even I solve with \$O(n)\$. I will appreciate any advice/answers.

You can see the problem, my code, and the scores on the Codility site, as well as here.

Problem Definition

  A zero-indexed array A consisting of N different integers is given. The array contains integers in the range [1..(N + 1)], which means that exactly one element is missing.
      Your goal is to find that missing element.
      Write a function:

class Solution { public int solution(int[] A); }

  that, given a zero-indexed array A, returns the value of the missing element.
      For example, given array A such that:

A[0] = 2
A[1] = 3
A[2] = 1
A[3] = 5

  the function should return 4, as it is the missing element.
      Assume that:
      N is an integer within the range [0..100,000];
      the elements of A are all distinct;
      each element of array A is an integer within the range [1..(N + 1)].
  Complexity:
      expected worst-case time complexity is O(N);
      expected worst-case space complexity is O(1), beyond input storage (not counting the storage required for input arguments).
      Elements of input arrays can be modified.

Solution

Here is the code I wrote as an answer:

public class Solution {

        public int solution(int[] A) {

            int N = A.length + 1;
            int total = N * (N + 1) / 2;

            for (int i : A) {

                total -= i;
            }

            return total;
        }
    }

@chillworld is correct in his observation that the issue is related to the int overflow causing the problems in the performance tests.
The more accurate solution to this is to use a long to set the score, instead of an int.
public class Solution {

    public int solution(int[] A) {

        long N = A.length + 1;
        long total = N * (N + 1) / 2;

        for (int i : A) {

            total -= i;
        }

        return (int)total;
    }
}

As for your code, I understand why you have variables with names like N and A, because those are the variable names used in the problem.
Those variable names do not conform to standard Java naming conventions though. They should be meaningful names, and start with a lower-case letter, like sum, and data.
the fact that my code below still uses N is not an indication that it is OK .... Do what I say, not what I do...

Update
Sometimes, the performance tricks in Java can be a PITA.

Whenever you can, declare your variables as final
declare your methods as final
declare variables and for-loop variables as final (if you use the enhanced-for semantics).
addition may, or may not be faster than subtraction.

Using this code:
class Solution {

    public final int solution(final int[] data) {

        final long N = data.length + 1;
        final long total = (N * (N + 1)) / 2;

        long sum = 0L;

        for (final int i : data) {

            sum += i;
        }

        return (int)(total - sum);
    }
}

Scores 100%

Update 2 final is not the issue:
public int solution(int[] data) {

    long N = data.length + 1;
    long total = (N * (N + 1)) / 2;

    long sum = 0L;

    for (int i : data) {

        sum += i;
    }

    return (int)(total - sum);
}

Also scores 100%


Separate Declaration and Definition of private Node implementation in template Linked List I was practicing implementing a template Linked List with a private node implementation.  I know one of the methods to define template class member functions is to keep the declaration and definition in the same header file.  When I tried to do this with the private createNode() function, I could not get it to work.

template<class T>
LinkedList<T>::createNode(const T &data)
{ ... }

I was wondering if there's a way to define the private function createNode() outside of the class declaration?  My current work around is to define this function within the class definition.  As I understand, this is a C++ style no-no though.

Generally speaking, I was wondering if there was anything else I could do with my implementation that could improve readability or make the  code more robust.  I'm open to any and all suggestions!  I suppose the first suggestion is to be consistent with where I place the pointer/reference modifiers...

#include <iostream>

template<class T>
class LinkedList
{
private:
    struct Node
    {
        T data;
        Node* next;
    };

    Node* head;
    Node* tail;
    int size;

    Node* createNode(const T &data)
    {
        Node* newNode = new Node;
        newNode->data = data;
        newNode->next = nullptr;

        return newNode;
    }

public:
    LinkedList();
    void display() const;
    void push(const T &data);
    void insert(const int pos, const T &data);
    void remove(const int pos);

};

template<class T>
LinkedList<T>::LinkedList() : head{ nullptr }, tail{ nullptr }, size{ 0 }
{
}

template<class T>
void LinkedList<T>::display() const
{
    Node* curNode = head;

    while (curNode != nullptr)
    {
        std::cout << curNode->data << std::endl;
        curNode = curNode->next;
    }

    std::cout << std::endl;
}

template<class T>
void LinkedList<T>::push(const T &data)
{
    Node* newNode = createNode(data);

    if (size == 0)
    {
        head = newNode;
        tail = newNode;
    }
    else
    {
        tail->next = newNode;
        tail = newNode;
    }

    ++size;
}

template<class T>
void LinkedList<T>::insert(const int pos, const T &data)
{
    if (pos < 0 || pos > size)
    {
        std::cout << "Index is out of range!" << std::endl;
    }
    else if (pos == size)
    {
        push(data);
    }
    else
    {
        Node* newNode = createNode(data);
        Node* prevNode = head;
        Node* curNode = head;

        int i = 0;
        while (i != pos)
        {
            prevNode = curNode;
            curNode = curNode->next;
            ++i;
        }

        prevNode->next = newNode;
        newNode->next = curNode;

        ++size;
    }
}

template<class T>
void LinkedList<T>::remove(const int pos)
{
    if (pos < 0 || pos > size)
    {
        std::cout << "Index is out of range!" << std::endl;
    }
    else if (size == 0)
    {
        std::cout << "List is empty!" << std::endl;
    }
    else
    {
        Node* prevNode = head;
        Node* curNode = head;

        int i = 1;
        while (i != pos)
        {
            prevNode = curNode;
            curNode = curNode->next;
            ++i;
        }

        prevNode->next = curNode->next;
        delete curNode;

        if (pos == size)
        {
            tail = prevNode;
        }

        --size;
    }
}

Your LinkedList does not manage its resources. You need a dtor, user-defined copy-ctor and user-defined assignment-operator at least.
Don't interact with the user from generic library code. Report any errors to the caller instead!
You probably want to provide the lists size to the public. Otherwise, consider whether optimizing removal of non-existent elements and insertion at the end by index is worth the overhead.
Only use std::endl if you really have to flush the stream. Remember flushing kills performance.
All your while-loops would be better off as for-loops.
createNode leaks memory if assignment to .data fails. Anyway, use aggregate-initialization, and you'll see that it's just not worth separating out into its own function:

auto p = new Node{data};

Regarding defining things in-class: There's nothing wrong with that, it's often even preferable, especially for templates.
If you define something outside a class in a header though, remember it won't automatically marked inline, meaning it must not be included in more than one compilation-unit.