N state probability in an M/M/2 queue I'm no ace when it comes to mathematical problems, so I've decided to go over some old college coursework and put it to code. The function works just fine and the returned values are spot on, but I'm wondering if there's a better way to solve this problem, perhaps more readable.

Problem:

  Restaurant 'A' has recently opened a 'drive through' service for its customers. The checkout has a mean arrival time of 20 per hour, with a service rate of 29 per hour. Due to the success of the new service, a second checkout has been opened.

  Rival restaurant, 'B', has launched an identical 'drive through' service, consisting of two checkout windows. This restaurant's mean arrival time is 19 per hour and the mean service rate is 17 per hour.

  In restaurant 'A', what is the probability that there will be exactly 2 customers in the queue?

  In restaurant 'B', what is the probability that the queue will be empty.

Formulas used

$$idle = P_0 = \frac{2\mu - \lambda}{2\mu + \lambda}$$
$$n\ state\ probability = P_n = \frac{1}{2^{n-1}}.\frac{\lambda}{\mu}^n.P_0$$

Where \$\lambda\$ is the arrival time, and \$\mu\$ is the service rate.

Code:

def n_state_probability(arrival_time, service_rate, n):
    """Return a value representing the probability an M/M/2 queue is in state n.

    To calculate the probability that the queue is empty, let n = 2.

    Arguments:
        arrival_time: mean arrival time per hour
        service_rate: mean service rate per hour
        n: amount of customers in the queue

    """
    idle = ((2 * service_rate) - arrival_time) / ((2 * service_rate) + arrival_time)
    probability = (1 / 2 ** (n - 1)) * ((arrival_time / service_rate) ** n) * idle
    if n == 2:
        return idle + probability + ((arrival_time / service_rate) * idle)
    return probability

if __name__ == '__main__':
    n_state_probability(19, 17, 2)
    # => 0.7760984526996147
    n_state_probability(20, 29, 4)
    # => 0.01377612256456733

Surely this is a trick question? The formulas only apply once the system has been running long enough that its behaviour approximates the stationary distribution. But the question explicitly says that A has "recently opened" and that B has "launched" its service. This suggests that the restaurants may not have been running long enough to approximate the stationary distribution, and so we may not be able to deduce anything about the probability of particular states.

Alternatively, the question is poorly phrased. Nonetheless, I would expect a solution to mention the assumption that the system has been running long enough for the formulas to be (approximately) valid.
Moving on, there's a second problem with the interpretation of the question. When it asks about "the queue", are we to include the customers currently being served (if any), or not?

In the docstring you write "n: amount of customers in the queue" which suggests that your interpretation is that the customers being served are in the queue.

But then you compute the answer to "what is the probability that the queue will be empty?" by adding \$P_0 + P_1 + P_2\$, which suggests that your interpretation is  that the customers being served are not in the queue.

I would expect you to make this decision one way and stick to it.
In the formulas, \$λ\$ must be the arrival rate, not (as claimed in the post) the arrival time.

This is clear from a simple dimensional analysis — if \$λ\$ were the arrival time then it would have dimensions of time, and so could not be added to the service rate \$μ\$ which has dimensions of time\$^{-1}\$.

Hence, in the code, the variable should be named arrival_rate, not arrival_time.
The docstring says, "mean arrival time per hour", but this is meaningless.
In fact, writing "per hour" at all is superfluous since the computation doesn't depend on the units. It would work just as well if the rates were "per minute", or "per day", or whatever, so long as they were consistent.
The interface to n_state_probability has a special case: "To calculate the probability that the queue is empty, let n = 2." The trouble with this is that now you can't use the function to compute \$P_2\$.

The Zen of Python says, "Special cases aren't special enough to break the rules". So I would remove the special case and let the caller compute \$P_0 + P_1 + P_2\$ by calling the function three times and adding the results.
The computation of \$P_n\$ involves exponentiating twice:

(1 / 2 ** (n - 1)) * ((arrival_time / service_rate) ** n) * idle

A small transformation to the equation means that you would only have to exponentiate once:

2 * ((arrival_time / (2 * service_rate)) ** n) * idle

For ease of checking the code against the formulas, I would suggest using the same names.

Revised code:

# -*- coding: utf-8 -*-

def n_state_probability(arrival_rate, service_rate, n):
    """Return the probability that an M/M/2 queue is in state n, assuming
    that the system has been running long enough that it is in the
    stationary distribution.

    Arguments:
        arrival_rate: mean arrival rate
        service_rate: mean service rate
        n: number of customers in the system

    """
    λ, μ = arrival_rate, service_rate
    assert 0 <= λ < 2 * μ # otherwise there is no stationary distribution
    P0 = (2 * μ - λ) / (2 * μ + λ)
    if n == 0:
        return P0
    else:
        return P0 * 2 * (λ / (2 * μ)) ** n

And then the answers are:

>>> n_state_probability(20, 29, 4)
0.01377612256456733
>>> sum(n_state_probability(19, 17, n) for n in range(3))
0.7760984526996149


Vector Sort Function I had heard about sorting for a while, but never actually gave it a try. So I decided to make an algorithm that would sort the members of a vector from least to greatest. Though this may seem pretty simple and/or silly, I actually put a lot of thought into it and wrote out tables/diagrams.

#include <iostream>
#include <vector>

namespace{
    typedef unsigned int uint;
}

std::vector<int> sort(std::vector<int>a)
{
    for (int i = 1; i < a.size(); ++i){
        for (int j = 0; j < a.size() - 1; ++j){
            if (a[j] > a[i]) std::swap(a[j], a[i]);
        }
    }

    return a;
}

int main()
{
    std::vector<int> a = { 17, 12, 1, 20, 4, 6, 94 };

    for (uint i = 0; i < a.size(); ++i){
        std::cout << sort(a)[i] << std::endl;
    }
}

I know there's probably some sort function part of the standard C++ library, but for a sort function from scratch, how efficient is this? What could I do to improve it?

It's not ideal to call sort() for each element.  The first call already gives you the sorted vector, so each additional call just needlessly overwrites it.  This approach would also be very slow if the vector were much larger.

Since you're already returning a vector, you just need to sort once and then print once:

int main()
{
    std::vector<int> a = { 17, 12, 1, 20, 4, 6, 94 };

    a = sort(a);

    for (auto& iter : a)
    {
        std::cout << iter << "\n";
    }
}

There's also no need for the typedef.  To help ensure you're using the proper type, you can use auto instead.  You should also replace the ints in sort() with auto.  Having higher compiler warnings would've alerted you to this type mismatch.


Generic Dictionary Equality Comparer I created the below comparer to allow me to use a generic dictionary as a key to another generic dictionary.

My GetHashCode implementation creates a hash based on all keys and their values; but I suspect its distribution could be improved?

Also my Equals method returns true only if the two arguments have exactly the same keys, with each key having exactly the same values.  However, potentially there's a more efficient approach to this comparison which I've neglected?

using System.Collections.Generic;
using System.Linq;

namespace MyCompany.Collections.Generic
{
    public class DictionaryEqualityComparer<TKey, TValue> : IEqualityComparer<IDictionary<TKey, TValue>>
    {
        readonly IEqualityComparer<TKey> keyComparer;
        readonly IEqualityComparer<TValue> valueComparer;

        public DictionaryEqualityComparer(): this(null, null){}
        public DictionaryEqualityComparer(IEqualityComparer<TKey> keyComparer, IEqualityComparer<TValue> valueComparer)
        {
            this.keyComparer = keyComparer ?? EqualityComparer<TKey>.Default;
            this.valueComparer = valueComparer ?? EqualityComparer<TValue>.Default;
        }

        public bool Equals(IDictionary<TKey, TValue> a, IDictionary<TKey, TValue> b)
        {
            if (a == null || b == null) return (a == null && b == null);                //if either value is null return false, or true if both are null
            return a.Count == b.Count                                                   //unless they have the same number of items, the dictionaries do not match
                && a.Keys.Intersect(b.Keys, keyComparer).Count() == a.Count             //unless they have the same keys, the dictionaries do not match
                && a.Keys.Where(key => ValueEquals(a[key], b[key])).Count() == a.Count; //unless each keys' value is the same in both, the dictionaries do not match
        }

        public int GetHashCode(IDictionary<TKey, TValue> obj)
        {
            //I suspect there's a more efficient formula for even distribution, but this does the job for now
            long hashCode = obj.Count;
            foreach (var key in obj.Keys)
            {
                hashCode += (key?.GetHashCode() ?? 1000) + (obj[key]?.GetHashCode() ?? 0);
                hashCode %= int.MaxValue; //ensure we don't go outside the bounds of MinValue-MaxValue
            }
            return (int)hashCode; //safe conversion thanks to the above %
        }

        private bool ValueEquals(TValue x, TValue y)
        {
            return valueComparer.Equals(x, y);
        }

    }
}

You said you built this specifically so you can use dictionaries as keys in other dictionaries?

In that case, you've got a problem. Consider the following demonstration:

var keyA = new Dictionary<string, int> { ["a"] = 4 };
var keyB = new Dictionary<string, int> { ["a"] = 4 };

var comparer = new DictionaryEqualityComparer<string, int>();
var dict = new Dictionary<Dictionary<string, int>, int>(comparer);
dict[keyA] = 10;

keyA["b"] = 5;
var x = dict[keyA];    // KeyNotFoundException: different hash
var y = dict[keyB];    // KeyNotFoundException: same hash, but not equal to original key

After modifying keyA, its hash-code is different, so a key lookup will fail. So far so good - this seems to be what you intended.

However, a lookup with keyB will also fail. Its hash-code is the same as keyA's original hash-code, so that part of the lookup succeeds, but because of hash collisions dict needs to compare it with the original key... which fails because keyA no longer has the same content.

That's why, generally speaking, the hash-code of an object should never change during its lifetime.

Also, the idea behind a hash-code is to have a quick, cheap way to filter out objects that are obviously not equal. Inspecting every single key and value seems fairly heavy-handed.

As for what you want to achieve... I suspect that will require a custom dictionary implementation, because you'll need to retain the original content of each key for future comparisons. At that point, I'd start thinking about why I even wanted to do this in the first place - there's likely a better solution for the original problem.


Coding Challenge : Build Tower refactoring code is it necessary In the interview you are asked to build a tower in a variable input e.g. n=3 .
The output should be as show below.
[
  '  *  ',
  ' *** ',
  '*****'
]

My solution is the following. Is this a good enough answer for the problem because I know people can solve it using Linq with less lines of code.
    public string[] PrintStairs(int n)
    {
        if (n == 1)
        {
            return new []{ " * " };
        }

        var result = new string[n];
        for (int row =0; row < n; row++)
        {
            var level = new StringBuilder();
            var midpoint = ((2 * n - 1) / 2);

            for (int col =0; col < 2 * n - 1; col++)
            {
                if((midpoint-row <= col) && (midpoint + row >= col))
                {
                    level.Append("*");
                }
                else
                {
                    level.Append(" ");
                }
            }
            result[row] = string.Concat(new string(level.ToString()));
        }
        return result;
    }

Linq Solution:
  public static string[] TowerBuilder(int nFloors)
  {
      return Enumerable.Range(1, nFloors).Select(i => string.Format("{0}{1}{0}", i == nFloors ? "" : new string(' ', nFloors - i), new string('*', 2 * i - 1))).ToArray();
  }

How did you determine the output ' # ' for the case n==1? I'd have expected it to just be '*'. If you're unclear about the specification for code you're writing in an interview, it's best to either ask for clarification or document your assumptions -- it's possible they deliberately handed you an unclear spec and are testing how you handle that.
If we take the interviewer request literally, you're still missing the [ ... ] formatting.
Naming
public string[] PrintStairs(int n)
This method is named as a verb indicating it's useful for its side effect, but it returns a value indicating it's useful for its return value. You should design APIs for one or the other but not both. Since your method is useful for its return value, a more appropriate name might be GenerateStairs or GetStairs.
Use more descriptive parameter names. numberOfStairs instead of n for example.
You use two different names, row and level, for different manifestations of the same concept: a step in your staircase. I'd unify these and call them rowIndex and rowBuilder respectively, or maybe stepIndex and stepBuilder.
Input validation
Your method crashes for negative values of n. If negative numbers are not valid inputs to the method, that fact should be documented by a method header and by throwing an ArgumentOutOfRangeException.
String APIs
result[row] = string.Concat(new string(level.ToString()));
This is a very complicated way of saying result[row] = level.ToString()! string.Concat is an API for concatenating strings, but you are only giving it one string which does nothing but give you the same string out again.
Explicit string constructors are intended to be used for creating strings from individual characters or from unmanaged code; here you're just giving it another string, which also does nothing but give you the same string out again.
General behaviour
var midpoint = ((2 * n - 1) / 2);
You are doing integer division here. Double-check your math because this expression simplifies to n - 1 for positive n -- were you intending for something more elaborate to happen with this calculation? A reader might wonder if there's a bug here.
n doesn't change inside the loop, so this whole line can be moved outside of it.
General formatting
Your spacing is inconsistent:
for (int row =0; row < n; row++)
if((midpoint-row <= col) && (midpoint + row >= col))
for (int col =0; col < 2 * n - 1; col++)
LINQ
Having everything on one line isn't necessarily a good thing. The LINQ solution is impenetrable when written like that. It deserves to be on multiple lines anyway for the sake of being able to read it properly:
return Enumerable.Range(1, nFloors)
    .Select(i => string.Format("{0}{1}{0}",
        i == nFloors
            ? ""
            : new string(' ', nFloors - i),
        new string('*', 2 * i - 1)))
    .ToArray();

Yes, it's technically LINQ, but the guts of that Select is what's doing all the heavy lifting.
The LINQ solution doesn't give the same output for n==1, which again begs the question where you got the idea that it should output ' # ' in the previous solution.
A more LINQ-y solution would have you build up the building blocks needed to write code that really is shorter. LINQ is all about operations on sequences; the thing that throws a wrench in any LINQ solution here is the fact that the output depends on how many things are in the sequence (the amount of space you use to pad the stairs depends on how many stairs there are), so you're probably going to have to enumerate sequences more than once.
Here's my LINQ-y take: start with a simple enumerator that gives you unformatted steps, from which you can take any arbitrary number of them:
private string GenerateStep(int stepIndex)
{
    if (stepIndex < 1)
        throw new ArgumentOutOfRangeException(nameof(stepIndex));
    return new string('*', stepIndex * 2 - 1);
}

private IEnumerable<string> EnumerateSteps()
{
    for (int stepIndex = 0; stepIndex < int.MaxValue; stepIndex++)
    {
        yield return GenerateStep(stepIndex);
    }
}

Now I can say EnumerateSteps().Take(3) to get:
[
  '*',
  '***',
  '*****'
]

Then define a method that takes a sequence of strings and centers them:
static class StringExtensions
{
    private static string CenterFormatted(string original, int totalWidth)
    {
        if (original == null)
            throw new ArgumentNullException(nameof(original));
        if (totalWidth < original.Length)
            throw new ArgumentOutOfRangeException(nameof(totalWidth));

        int paddingNeededOnLeft = (totalWidth - original.Length) / 2;
        int paddingNeededOnRight = totalWidth - original.Length - paddingNeededOnLeft;

        return String.Concat(
            new string(' ', paddingNeededOnLeft),
            original,
            new string(' ', paddingNeededOnRight));
    }

    public static IEnumerable<string> CenterFormatted(this IEnumerable<string> sequence)
    {
        // Exercise: how will you check for a null input sequence here? If you check it here and throw ArgumentNullException, will it actually throw when you call CenterFormatted?
        int largestStringLength = sequence.Max(s => s.Length);
        foreach (string original in sequence)
        {
            yield return CenterFormatted(original, largestStringLength);
        }
    }
}

Now you have two orthogonal and reusable things: a thing that generates steps and a thing that centers sequences of strings. You can compose these quite concisely in a LINQ statement to solve the original problem:
EnumerateSteps().Take(3).CenterFormatted().ToArray()
which gives me
[
  '  *  ',
  ' *** ',
  '*****'
]


Is it advisable to integrate SQL statements in the JSP pages that are not displayed to the user but handled at the back? I have a signup form in which the user has to enter the email address and after some quick asynchronous processing at the backend I have to tell the user that whether the email address is already registered or not.

Below is a sample example of the JSP page that I have created,

<input type="email" id="inputEmail" placeholder="*Email" name="inputEmail" required="true" onchange="checkDuplicate();"
    onpaste="this.onchange();" oninput="this.onchange();" onsubmit="this.onchange();" autocomplete="off"/>

Now the at the backend the check_email.jsp page will be like this,

<%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %>
<%@ taglib uri="http://java.sun.com/jsp/jstl/sql" prefix="sql"%>

<c:set var="email" value="${param['email']}"/>

<sql:setDataSource var="db" driver="com.mysql.jdbc.Driver" user="root"  password="root" url="jdbc:mysql://localhost:3306/ercafe"/>
<sql:query dataSource="${db}" sql="SELECT * FROM user_details" var="result" />

<c:forEach var="row" items="${result.rows}">

    <c:if test="${row.user_email_id eq email}">
        <c:out value="This email is already registered"/>
    </c:if>

</c:forEach>

I know that it is not advisable to include the database interaction in the JSP page but as this page would be handled in the backend without any direct interaction from the user I'm very tempted to use check_email.jsp as above.
Can anyone tell me how to handle the check_email.jsp by the ideal way?

General advice

  I know that it is not advisable to include the database interaction in
  the JSP page but as this page would be handled in the backend without
  any direct interaction from the user I'm very tempted to use
  check_email.jsp as above.

It's a good thing when you know that something is wrong to search for a better way to do it. This is an important part of having good code in my opinion.

What you must not do, is code something that you know is not right, but look like having no impact of security or side effects. There is a chance that you won't be the last programmer on this code. What will happen if he decide that the view is more publicly accessible ? (In this case not much since from my understanding, JSP tag are interpreted server-side)

There is one thing you know for sure, that if this code is in the server/controller/service, it will be where he belong!

Re-usability

This SQL request looks to me like a request that could be re-use in different in an application. Will paste it everywhere you need it or would ratter call a service that will return the result of the query ? The second option look better to me.

I can't provide a good example of a service without knowing what framework you use. There is plenty of resource on the internet that could help you, I could point you some tutorials if I could know what your set-up is.

Quick note

In your JavaScript you have var url = "../ajaxPages/check_email.jsp?email="+email;. Are those pages suppose to be responses to AJAX request ? If so, you should know that you can probably create a controller that do respond to AJAX request. In my opinion, this will help you keep all the important logic in the controller and remove some JSP pages that I guess are not doing what they should be doing.


Implementing two stacks using single array in java Please review the code

package com.gmail.practice;

import java.util.Arrays;

public class StacksForTwo {

    int size;
    int[] stack;
    int top1;
    int top2;

    public StacksForTwo(int arraysize)
    {
        size = arraysize;
        stack = new int[size];
        top1 = -1;
        top2 = size;
    }

    public void push1(int x)
    {
        if(top1 < top2-1)
        {
        top1++;
        stack[top1] = x;
        }else{
            System.out.println("stackoverflow");
        }
    }

    public void push2(int y)
    {
        if(top1 < top2-1)
        {
            top2--;
            stack[top2] = y;
        }else{
            System.out.println("stack overflow");
        }
    }

    public void pop1()
    {
        if(top1 >= 0)
        {
            top1--;
            System.out.println("The popped out number is"+" "+stack[top1+1]);
        }else{
            System.out.println("stack underflow");
        }
    }

    public void pop2()
    {
        if(top2 < size)
        {
            top2++;
            System.out.println("The popped out number is"+" "+stack[top2+1]);
        }else{
            System.out.println("stack underflow");
        }
    }

    public void display()
    {
        System.out.println(Arrays.toString(stack));
    }

    public static void main(String[] args)
    {
        StacksForTwo sft = new StacksForTwo(10);
        sft.push1(4);
        sft.push1(5);
        sft.push1(3);
        sft.push1(2);
        sft.push2(6);
        sft.push2(4);
        sft.display();
        sft.push2(8);
        sft.push1(2);
        sft.push2(6);
        sft.push2(4);
        sft.push2(8);
        sft.display();
    }
}

I would go for

interface Stack {
    boolean isEmpty();
    int pop();
    void push(int x);
}

And then make a class providing two Stacks.

Also create a counter to detect when both stacks are full. This can be done with an AtomicInteger (thread-safeness) counting the free array slots.

public class StackPair {
    public final Stack firstStack = new Stack { ... };
    public final Stack secondStack = new Stack { ... };
    public StackPair(int capacity) { ... }

In StackPair the single array and an AtomicInteger freeEntries. In both Stack implementations access to the array and freeEntries.


Text-based Tic Tac Toe with DRY and PEP-8 This is a basic Tic-Tac-Toe game with a minimal interface. The only concessions to ease of use are input validation (7 lines) and 1-indexed rows and columns (a few bytes). I've been trying to tighten it up while still maintaining readability and PEP-8 style.

I'm pretty happy with most of it, except for printing the board: I'd like to do it at the beginning, between each move, and at the end, but I don't see how to manage that without either repeating the formatted print() call or turning it into a function and calling it twice. As you can see, my current compromise omits the empty starting board in favor of the more interesting ending board.

This printing issue is my focus at the moment, but I'm also interested in any other suggestions for improvement.

board = [['*' for i in range(3)] for j in range(3)]

def askmove(piece):
    while True:
        try:
            x,y = map(int, input('\n> '))
            if board[x-1][y-1] == '*':
                board[x-1][y-1] = piece
            else:
                continue
        except (ValueError, IndexError):
            pass
        else:
            break
    print('', *(' '.join(row) for row in board), sep='\n')
    if ([piece] * 3 in board or
        (piece,) * 3 in zip(*board) or
        all(board[i][i] == piece for i in range(3)) or
        all(board[i][2-i] == piece for i in range(3))):
        print(piece, 'wins.')
        return True
    elif all(p!='*' for row in board for p in row):
        print('Tie.')
        return True

while True:
    if askmove('X') or askmove('O'):
        break

Input validation

Input validation is not really related to Tic-Tac-Toe, but is shared among all console programs.

The code to get the correct coordinates should be moved into a function in its own file to reuse it in the future and make the tic-tac-toe script even shorter.

Names

askmove does a complete turn, so I would call it: make_turn

Refactor

The check for the winning position is very much isolated from the rest of the program. I think the program would gain a lot of readibility if you wrote a is_won function.


Simulations and game code I will like to improve my code in Java involving creating simulations and games. This is a problem-solving code so far in interest and percentage calculations.

import javax.swing.JOptionPane;
class interest {
    public static void main(String[] args){
        String name = JOptionPane.showInputDialog( "Item" );
        String message = String.format( "Word is, %s, is Items", name );

        JOptionPane.showMessageDialog( null, message );

        String firstNumber = JOptionPane.showInputDialog( "Enter QuantitySold Double " );
        String secondNumber = JOptionPane.showInputDialog( "Enter UnitCost Double " );
        String thirdNumber = JOptionPane.showInputDialog( "Enter OriginalPrice Double " );
        String fourthNumber = JOptionPane.showInputDialog( "Enter lowedPercentagePrice Double " );

        Double QuantitySold = Double.parseDouble( firstNumber );
        Double UnitCost = Double.parseDouble( secondNumber );
        Double OriginalPrice = Double.parseDouble( thirdNumber );
        Double LowedPercentagePrice = Double.parseDouble( fourthNumber );
        Double TotalCost = QuantitySold * UnitCost ;

        JOptionPane.showMessageDialog( null, " TotalCost is " + TotalCost, "TotalCost of Two Doubles",
        JOptionPane.PLAIN_MESSAGE );

        Double PercentagePrice = LowedPercentagePrice / 100;
        Double PricePercentage = OriginalPrice * PercentagePrice ;
        Double TotalPrice = OriginalPrice - PricePercentage;

        JOptionPane.showMessageDialog( null, " TotalPrice is " + TotalPrice, "TotalPrice of Two Doubles", JOptionPane.PLAIN_MESSAGE );
    }
}

class interest

In Java, class names are CasedLikeThis (pascal cased). Interest would be a proper name.

Name those strings more appropriately.

String firstNumber = JOptionPane.showInputDialog( "Enter QuantitySold Double " );

is a case of inappropriate naming. firstNumber is not a numeric value, it's a string. It's also not a "first string", it's a string that is supposed to represent the sold quantity, which is to be parsed as a double.

String rawQuantitySold = JOptionPane.showInputDialog("Enter QuantitySold Double");

would be a first step towards improvement.

You don't check for input validity. What if the user inputs some garbage, and not a valid string representation of a numeric value? Consider extracting the input bits, and forming a loop that continues for as long as the user inserts invalid data.

NumericType getNumericValue(String messageForUser)
{
     while (true)
     {
          String rawInput = JOptionPane.showInputDialog(messageForUser);
          if (canBeParsedToNumericType(rawInput))
          {
                return parseToNumericType(rawInput);
          }
          else
          {
               showMessage("Value has an invalid format. Please enter a valid value.");
          }
     }
}

and use it like:

Double quantitySold = getNumericValue("Enter the quantity that's been sold (floating point value).");

This also makes you get rid of the extra temporary strings there.

Tip! The else branch there is unnecessary, due to the fact that return will return immediately. You may or may not want to remove it.

Consider improving the messages you're showing to your users.

  Enter OriginalPrice Double

is not exactly a properly formatted and user aimed message.

  Enter the original price.

With, maybe, some info about the format that it has to have, and/or some examples would be a huge improvement.

Keep naming consistent.

String fourthNumber = JOptionPane.showInputDialog( "Enter lowedPercentagePrice Double " );
Double QuantitySold = Double.parseDouble( firstNumber );

Why is QuantitySold pascal cased, and fourthNumber camel cased? quantitySold should be your choice there.

Improve your formatting.

JOptionPane.showMessageDialog( null, message );

could become

JOptionPane.showMessageDialog(null, message);

Also,

class interest
{
public static void main(String[] args){

could be turned into

class Interest {

    public static void main(String[] args) {

and you should take a look at where you drop your braces, as well as where you drop random empty lines. If you need to separate your code in pieces with empty lines for readability, chances are you have to extract methods from there.

Consider, for example, extracting

Double PercentagePrice = LowedPercentagePrice / 100;
Double PricePercentage = OriginalPrice * PercentagePrice;
Double TotalPrice = OriginalPrice - PricePercentage;

into a function that calculates the total price based on some parameters that you set to it.

Double totalPrice = calculateTotalPrice(...);

Rule of thumb: if you have to split your functions in chunks using empty lines or verbose comments, you should first take a look if you couldn't extract those chunks in separate functions.


Fastest way to count combinations of values from two lists/arrays I have a file with mostly 0 and 1. For each line I would like to compare it against all others (without the redundancy) and count the different combinations. It is slightly more complicated as there are also "N" in the matrix when there is missing data. For example if the two lines are:

line1 = "0 0 1 0 1"
line2 = "0 1 1 0 1"

Then I would like to create a two by two table like this:

    0   1
 0| 2 | 1 |
 1| 0 | 2 |

At the moment I am doing something like this:

line1_list = line1.split()
line2_list = line2.split()

To get the counts I have tried two options. The first if using a for loop:

tab_dict = defaultdict(int)
for i in xrange(0,len(line1_list)):
    key = line1_list[i]+"-"+line2_list[i]
    tab_dict[key] += 1

And the second is using a combination of map and zip:

tab_dict = Counter(map(lambda x:x[0]+"-"+x[1],zip(line1_list,line2_list)))

The first option seems to be the fastest but I was wondering if there is a faster way because the matrix I have is very big and it will take a long time to run. I have tried to look for libraries to do this but have not found anything yet.

I would drop construction of string and just do:

from itertools import izip

line1 = "0 0 1 0 1"
line2 = "0 1 1 0 1"

line1_list = line1.split()
line2_list = line2.split()

tab_dict = Counter(izip(line1_list, line2_list))

construction of tuple if faster than string, you remove unneeded operation, and making a table from a tuple of two if pretty much the same as making it from a complex string key, I would say that making it out of tuple of (x,y) is more logical.


Binary search modification for "swaped" array I was making an algorithm for a task I found in a book. It says that there is sorted array, that was swapped so it looks like "4567123", and was proposed to use binary search modification.

Below is my solution in java.

The thing is, I'm a bit cringed that I can't process cases for array size of 2, 3 generically (for case of size 1 it's obvious that it'll require a specific case). Not that I need a solution that'll do that generically for 2, 3 array size cases. But I more like to know, is it a problem. Also I addressed a problem when array wasn't swaped at all. And I'm not sure that I should've done this, if preconditions are clearly specified. Btw binary search doesn't check that it's input is sorted.

So, more what I need is not code review itself, but more of advise, is my hardcoded cases and support for nonswaped array is good or bad. I myself believe that cases are fine, as algorithm compares 2 values, and thus requires at least 2 values to be compared, also it searches for the "peak and drop". But I'll probably try to modify it so it won't cover non-swaped array case and generically work for 2, 3 size array.

//file SwapedArraySearch.java\
public class SwapedArraySearch {
    public static int search(int[] arr) {
        switch (arr.length){
            case 1:
                return arr[0];
            case 2:
                return arr[0] > arr[1]?arr[1]:arr[0];
            case 3:
                return arr[0] > arr [1] ? (arr[1] > arr[2]?arr[2]: arr[1]):
                                            (arr[0] > arr[2]?arr[2]: arr[0]);
        }
        int x = arr[0];
        int n = arr.length/2;
        int prevn = arr[n] > x ? arr.length -1 : 1;
        int t;
        while(prevn != n) {
            if (x < arr[n]) {
               if (arr[n] > arr [n + 1] )
                   return arr[n+1];
                t = n;
                n = n + (prevn - n)/2;
                prevn = t;
            } else {
                if (arr[n - 1] > arr[n])
                    return arr[n];
                t = n;
                n = prevn + (n - prevn)/2;
                prevn = t;
            }
        }
        return arr[0] > arr[1] ? arr[arr.length - 1] : arr[0];
    }

    public static void main(String... args) {
        int[] arr = new int[args.length];
        for (int i = 0; i < args.length; i++)
            arr[i] = Integer.valueOf(args[i]);
        System.out.println(search(arr));
    }
}

EDIT: the task is to find the lowest element

EDIT2: after much more thinking I boiled down my search function to this:

x = arr[arr.length - 1]
a = 0
b = arr.length
while b - a > 2
 if x > arr[(a+b)/2]
  b = (a+b)/2
 else
  a = (a+b)/2
return arr[(a+b)/2]

the key point to simplification was to understand that middle element can always be calculated as (a+b)/2.

General Advise

Your methods are difficult to read because you chose to use predominantly single character variable names. Multi-character descriptive variable names are much easier to associate with a value, making the code magnitudes easier to read & maintain. Also, when implementing a binary search it is typical to use hi, lo & mid as variable names.
Whenever you nest a ternary statements, this should be an immediate red flag, that you are not writing clear & maintainable code. As soon as you write a nested ternary statement I would advise you to pause, and consider what functionality you are trying to achieve and consider different ways to express that functionality. I'm not going to say that nested ternary statements are always a sign that your doing something wrong, but nested ternary statements is always a sign you should stop and think about what your doing.

Algorithmic Advise

I am assuming the algorithm is locating the largest element in a semi-sorted array that has two (and only two) distinctly sorted sequences with a pivot index contains largest value in the array.

The three conditions in your switch case are, generally doing the same thing. Each case is returning the largest element in a the array, but can't use the loop because the array is too small. We can cover these conditions in the final return.
You should also check for general error cases such as null or the empty set (new int[0]) being passed into the function.
Since the logic for checking if the current value of the binary search is the pivot index contains a lot of logic to do correctly, I would create a method isPivotIndex().

Putting all that together, your solution could look something like this.

Java Code: (ideone example link)

int findLargestValueInSemiSortedArray(int arr[]) {
  if(arr==null || arr.length==0)
    throw new IllegalArgumentException();

  int hi  = arr.length; /* exclusive upper bound */
  int low = 0;          /* inclusive lower bound */
  int mid;
  while(low < hi) {
    mid  = low/2 + hi/2;
    if(isPivotIndex(arr,mid))
      return arr[mid];
    if(arr[hi-1] < arr[mid])
      low = mid + 1;
    else
      hi  = mid;
  }
  /* Array was actually sorted, either ascending or descending    */
  /* Note that return this covers corner cases of arr.length <= 3 */
  return max(arr[0], arr[arr.length-1]);
}

int max(final int a, final int b) {
  return (a > b) ? a : b;
}

boolean isPivotIndex(final int[] arr, final int index) {
  int b = arr[index];
  int a = (index > 0) ?            arr[index-1] : Integer.MAX_VALUE;
  int c = (index < arr.length-1) ? arr[index+1] : Integer.MAX_VALUE;
  return a <= b && b > c;
}

The above is much more readable, has fewer lines of code, and is easier to check for correctness.

It immediately throws an exception on invalid input.
It uses isPivotIndex() method to isolate complex logic & maintain a single level of abstraction.
It uses a clever final return to handle corner cases.

Let's consider how the final return covers the corner cases in your original switch statement.

Case 1: (arr.length == 1)

max(arr[0],arr[arr.length-1]) will compare the same values and return arr[0].

Case 2: (arr.length == 2)

max(arr[0],arr[arr.length-1]) will evaluate exactly the same as your ternary statement.

Case 3: (arr.length == 3)

max(arr[0],arr[arr.length-1]) will return the larger of arr[0] and arr[2]. If arr[1] happened to be the max value it would have been the pivot index in the binary search loop and the method would have already returned.


Accessing argparse arguments from the class I have the following sample code running unit tests:

#!/usr/bin/env python3

import sys
import unittest
import argparse

class ParentTest(unittest.TestCase):
    None

class Test1(ParentTest):
    def test_if_verbose(self):
        import __main__ # FIXME
        print("Success!") if __main__.args.verbose else "" # FIXME

class Test2(ParentTest):
    None

if __name__ == '__main__':
    # Parse arguments.
    parser = argparse.ArgumentParser(add_help=False)
    parser.add_argument("-?", "--help",     action="help",                        help="show this help message and exit" )
    parser.add_argument("-v", "--verbose",  action="store_true", dest="verbose",  help="increase output verbosity" )
    parser.add_argument('files', nargs='*')
    args = parser.parse_args()

    print(args.verbose)

    # Add tests.
    alltests = unittest.TestSuite()
    alltests.addTest(unittest.makeSuite(Test1))
    alltests.addTest(unittest.makeSuite(Test2))

    result = unittest.TextTestRunner(verbosity=2).run(alltests) # Run tests.
    sys.exit(not result.wasSuccessful())

and I'd like to access command-line argument values from the class.

In here it's suggested that using __main__ isn't really a good approach.

Is there any better way of doing it, where individual tests could have access to argument values which were parsed already in main?

Testing:

python3 test.py
python3 test.py -v

You could do this by making the test classes take args as parameter, and crafting a custom make_suite method instead of unittest.makeSuite, like this:

class ParentTest(unittest.TestCase):
    def __init__(self, methodName='runTest', args=None):
        super().__init__(methodName)
        self.args = args

class Test1(ParentTest):
    def test_if_verbose(self):
        print("Success!") if self.args.verbose else ""

class Test2(ParentTest):
    pass

if __name__ == '__main__':
    parser = argparse.ArgumentParser(add_help=False)
    parser.add_argument("-?", "--help",     action="help",                        help="show this help message and exit" )
    parser.add_argument("-v", "--verbose",  action="store_true", dest="verbose",  help="increase output verbosity" )
    parser.add_argument('files', nargs='*')
    args = parser.parse_args()

    print(args.verbose)

    def make_suite(testcase_class):
        testloader = unittest.TestLoader()
        testnames = testloader.getTestCaseNames(testcase_class)
        suite = unittest.TestSuite()
        for name in testnames:
            suite.addTest(testcase_class(name, args=args))
        return suite

    # Add tests.
    alltests = unittest.TestSuite()
    alltests.addTest(make_suite(Test1))
    alltests.addTest(make_suite(Test2))

    result = unittest.TextTestRunner(verbosity=2).run(alltests) # Run tests.
    sys.exit(not result.wasSuccessful())

The sample output is the same as the original code:

$ python t2.py ; python t2.py -v
False
test_if_verbose (__main__.Test1) ... ok

----------------------------------------------------------------------
Ran 1 test in 0.000s

OK
True
test_if_verbose (__main__.Test1) ... Success!
ok

----------------------------------------------------------------------
Ran 1 test in 0.000s

OK

The code in my answer was inspired by these posts:

https://stackoverflow.com/a/17260551/641955
http://eli.thegreenplace.net/2011/08/02/python-unit-testing-parametrized-test-cases/


Is this use of (let ... and ...) in OCaml poor style? In other languages, I prefer to arrange source files so that simpler and more widely useful concepts are introduced before implementation details, and I try where possible to make complex implementation details top-level functions so that I can separate them out and let the main function read like a high-level algorithm description.

OCaml obviously has an interface/source distinction, but even with a sparse well-documented interface like

type t
(** Blah blah blah *)

val important_operator : t -> t -> t
(** Blah blah blah *)

I've been tempted to do things like

let rec important_operator a b =
  let a', b' = foo a b in
  let a', b' = iterate_until_convergence (=)
                 (fun (a, b) -> bar (baz a b))
                 (a', b') in
  merge a', b'

(* Implementation details *)

and foo a b = ...

and bar x = ...

and baz a b = ...

and merge a b = ...

Is this poor style?  Does it affect the ability of IDE-users to navigate source files?

It's not idiomatic OCaml indeed.

"and" is an effective way to say "watch out for mutual recursion".
OCaml encourages you to use nested functions instead! As The Structure of OCaml programs says: "Nested functions are, however, very useful and very heavily used in OCaml."

You would write this code as follows:

let important_operator a b =
  let foo a b = ... in
  let bar x = ... in
  let baz a b = ... in
  let merge a b = ... in
  let a', b' = foo a b in
  let a', b' = iterate_until_convergence (=)
                 (fun (a, b) -> bar (baz a b))
                 (a', b') in
  merge a', b'

Of course, you don't need to only use nested functions, you can also define bar or baz before defining important_operator. OCaml tends to only use functions that were defined "earlier", so using and for this is abusing the system.


Before/after jQuery highlighter I have a webpage that shows shorthand code on the left, and its long form on the right. When you hover over code on the left (signified with a space-separated array in the data-from attribute), it uses jQuery to highlight corresponding code on the right (signified with a space-separated array in the data-to attribute), and vice-versa. It's working great, but it feels very long.

Script

function containsAny(haystack, needles) {
    for(needle in needles)
        if (haystack.indexOf(needles[needle]) >= 0)
            return true;
}
$(document).ready(function() {
    /* from to */
    $("[data-from]").each(function(i, e) {
        var $e = $(e),
        froms = $e.data("from").toString().split(/\s+/); // must do toString in case we get an int
        $e.hover(function() {
            $e.addClass("hilighted");
            $("[data-to]").each(function(i2, e2) {
                var $e2 = $(e2),
                tos = $e2.data("to").toString().split(/\s+/);
                if (containsAny(tos, froms))
                    $e2.addClass("hilighted");
            });
        },
        function() {
            $("[data-to]").each(function(i2, e2) {
                $e.removeClass("hilighted");
                var $e2 = $(e2),
                tos = $e2.data("to").toString().split(/\s+/);
                if (containsAny(tos, froms))
                    $e2.removeClass("hilighted");
            });
        });
    });

    /* to from */
    $("[data-to]").each(function(i, e) {
        var $e = $(e),
        tos = $e.data("to").toString().split(/\s+/);
        $e.hover(function() {
            $e.addClass("hilighted");
            $("[data-from]").each(function(i2, e2) {
                var $e2 = $(e2),
                froms = $e2.data("from").toString().split(/\s+/);
                if (containsAny(froms, tos))
                    $e2.addClass("hilighted");
            });
        },
        function() {
            $("[data-from]").each(function(i2, e2) {
                $e.removeClass("hilighted");
                var $e2 = $(e2),
                froms = $e2.data("from").toString().split(/\s+/);
                if (containsAny(froms, tos))
                    $e2.removeClass("hilighted");
            });
        });
    });
});

SSCCE HTML

Before

*:link {
    color: blue;
}

button,
input[type=("button","submit","reset")] {
    color: match(*:link);
}

After

*:link,
button,
input[type="button"],
input[type="submit"],
input[type="reset"] {
    color: blue;
}

Full use case

How can I make the JavaScript shorter (or at least more OO) without sacrificing functionality? I'm sure it can be shorter or more OO because it's so redundant, but I can't figure how.

Here's your code, and I've pointed out some potential problems:

for(needle in needles)
    if (haystack.indexOf(needles[needle]) >= 0)
        return true;

While this is possible, I suggest you use {} instead. This avoids ambiguity, and future danger, especially when you'll be adding more functionality beyond just a single line.

Additionally, for-in runs over prototype properties. If you happen to have a property that is similarly named as a native property in the prototype, you'll be in trouble. To avoid that, check using hasOwnProperty before proceeding with the code.

$(document).ready(function() {

Probably not better, but there's a shorthand for this: $(function(){...})

$("[data-from]").each(function(i, e) {

I'd probably avoid using attribute selectors. They're known to be slow, depending on the parser implementation. I suggest you stick to classes.

froms = $e.data("from").toString().split(/\s+/); // must do toString in case

A shorthand to convert stuff to string is to do '' + to it. Not so verbose, but it's somewhat elegant and short.


Getting a substring of a string in JavaScript I can have one of the following strings:

var username = "i:0#.f|myprovider|domain\\myuser"
var username2 = "myprovider|domain\\myuser"
var username3 = "myuser"

I always want only the myuser part of the string!

I tried the following in JS

var n = username.lastIndexOf('|');
var domainandusername = username.substring(n + 1);
var m = domainandusername.lastIndexOf('"\\"');
var username = domainandusername.substring(m + 1);

I have tried it within a simple page with JS and it seems to work. But is this a good way to solve this? I mean is it a good practice even to do this for string username2 and username3 where I know that they, for example, do not have a | and \\?

You don't need to over-complicate things. The myuser part will always come at the end of the string, after \\ if it's there.

All you need is this:

username.split("\\").pop();

Even if the username is entered like the third version, the myuser will be returned.


Is this merge sort code good enough? public class MergeSort {

    public static void sort(int[] array, int lo, int hi) {
        if (hi <= lo) {
            return;
        }

        int mid = lo + (hi - lo) / 2;
        sort(array, lo, mid);
        sort(array, mid + 1, hi);
        mergesort(array, lo, mid, hi);
    }

    private static void mergesort(int[] array, int lo, int mid, int hi) {
        int[] aux = new int[array.length];
        for (int i = 0; i < array.length; i++) {
            aux[i] = array[i];
        }
        int i = lo;
        int j = mid + 1;
        for (int k = lo; k <= hi; k++) {
            if (i > mid) {
                array[k] = aux[j++];
            } else if (j > hi) {
                array[k] = aux[i++];
            } else if (aux[i] > aux[j]) {
                array[k] = aux[j++];
            } else {
                array[k] = aux[i++];
            }
        }
    }

    public static void main(String[] args) {
        int[] a = { 3, 1, 5, 1, 8, 23, 55, 54, 10, 12, 5 };
        MergeSort.sort(a, 0, a.length - 1);
        show(a);
    }

    private static void show(int[] a) {
        for (int i = 0; i < a.length; i++) {
            System.out.println(a[i]);
        }
    }

}

It took me a whole day and a half to understand the code but I did a quick test and it worked. One thing I am concerned with is that the coursera class I am taking does it differently, instead of creating aux in mergesort they create aux in a helper class and pass around BOTH arrays in the sort and mergesort method. It looked like this:

private static void sort(Comparable[] a, Comparable[] aux, int lo, int hi) {
    if (hi <= lo) return;
    int mid = lo + (hi - lo) / 2;
    sort(a, aux, lo, mid);
    sort(a, aux, mid + 1, hi);
    merge(a, aux, lo, mid, hi);
}

/**
 * Rearranges the array in ascending order, using the natural order.
 * @param a the array to be sorted
 */
public static void sort(Comparable[] a) {
    Comparable[] aux = new Comparable[a.length];
    sort(a, aux, 0, a.length-1);
    assert isSorted(a);
}

and when it got to the merge sort method it finally copies the aux array, what is the point of this route?  It looks like it just takes more code.

General comments

you have 'good' mid-finding code: int mid = lo + (hi - lo) / 2; This is fancy code, and, if you have not learned why it is done this way, then you should read up on the midpoint bug in Java.
your algorithm generally looks sound. It works

The basic premise of your sort is good. The code is neat, and, if you know the merge-sort, it is readable, and does the right sort of things at the right times.

With just your code in front of me, I would say 'good job'. To make it better, I would recommend that:

you make your code conform to the standard usage in Java, where you would have two methods:

MergeSort.sort(int[] data);
MergeSort.sort(int[] data, offset, length);

Java typically uses a 'length' argument, instead of a 'high' argument. In the case of the MergeSort, the 'high' variation is more useful, so I would recommend making your current method private, and then adding a new recursion-wrapping frontend to the code that makes it conform to typical Java method calls.

About the aux

This is the second reason to consider wrapping your current sort-entry method in to a calling method.... something like:

public static void sort(int[] data) {
    sort(data, 0, data.length);
}

public static void sort(int[] data, int offset, int length) {
    int[] aux = new int[data.length]; // smarter implementation can be used for if length != data.length
    recursiveSort(data, aux, offset, offset + length);
}

Where the method recursiveSort is the method you currently have as sort.

The reason for doing the aux in the calling method is for memory efficiency. At some point you will need to do the final merge, which will require the entire data array to fit in to a memory 'copy', the aux. You will also do many more smaller merge sorts requiring a smaller size of memory. If you are going to need the large space anyway, you may as well create it in the beginning, and reuse it many times, which is better than creating many small aux which never get reused.


Need some advice and feedback on my C code calling Bash functions My intent is to use Bash functions defined in functions.sh in a C program. I am doing this so that I don't have to rewrite the Bash functionality again in C. I want to use one common library for functions I need. Here is my C code and functions.sh.

#include <stdlib.h>
#include <sys/wait.h>
#include <stdio.h>

void shellCall(char * command)
{
    int status, commandexitcode=0;

    if(command == NULL)
    {
        printf("NULL command string sent\n");
        exit(1);
    }

    status = system(command);

    if(status == -1)
    {
        printf("Error during call, value %d\n", status);
        exit(1);
    }

    if (WIFEXITED(status))
    {
        commandexitcode =  WEXITSTATUS(status);
        if (commandexitcode != 0)
        {
            printf("non zero exit code: %d\n", commandexitcode);
            exit(1);
        }
    }
    else if (WIFSIGNALED(status))
    {
        printf("killed by signal: %d\n", WTERMSIG(status));
        exit(1);
    }
    else if (WIFSTOPPED(status))
    {
        printf("stopped by signal: %d\n", WSTOPSIG(status));
        exit(1);
    }
    else if (WIFCONTINUED(status))
    {
        printf("continued, unexpected..bailing\n");
        exit(1);
    }
}

int main(int argc,char **argv)
{
    char  command[100]; //command
    int i = 0;
    for (i = 1 ; i <= 10 ; i++)
    {
         sprintf( command, "source $PWD/functions.sh ; i=%d ; myfunc %d", i,i );
         shellCall(command);
    }
}

Please note that here I have only shown a very simple example of the way I am using functions in functions.sh. In reality functions.sh has many functions, some of which implement decent logic.

#!/bin/sh
# functions.sh

myfunc()
{
    echo "I received $1"
}

Question: I need feedback on the following aspects.

Is it a bad programming practice (something that will make experienced and senior developers mad when they see this) to use system() calls to call functions in functions.sh as I have done?
What are the reasons (technical and other) for which it is considered a bad programming practice?
What changes should I make to my C code so that experienced and senior developers will find it acceptable?

Comments

Because your question is marked clean-code and I see that you have commented your declaration of the variable command with the comment command, let me make a note about comments. There are good comments, and bad comments; in my experience, a very high numbers of comments written by many developers are bad. For a thoughtful exposition of comments, see Bob Martin's chapter on them in Clean Code (an invaluable book). In short, if you are going to insert a comment, make sure it actually adds something. Then, make sure you can't add that information into the code itself, by renaming variables, etc. Then, if you can't, carefully craft your comment.

Functions

Because you are asking about the general cleanliness of your code, let me advice that you use smaller functions. There are many benefits to small functions, which I will not get into here; many of the best software books discuss this, including Martin's book, mentioned above.

Magic Numbers

Replace numbers like -1 and 10 in your code with a constant that has an explicit name explaining these numbers. This increases the readability and maintainability of your code.

Stdout and Stderr

In Unix, C and C-based languages, you have three streams attached to your process by default: standard in, standard out, and standard error. Don't write error messages to standard out—write them to standard error.

Calling Bash from C

Consider that calling Bash...

...makes your code much, much less portable, by tying it down to a system with Unix utilities—and more specifically than that, one that has Bash—and more than that, one that has a version of Bash which supports all the features you decide to use.
...makes your code harder to test

by creating external dependencies. If you are not familiar with the problem of dependencies, try reading up on it. Roy Osherove, in The Art of Unit Testing, and Michael Feathers, in Working Effectively with Legacy Code, both have much discussion of it.
Bash scripts do not have the debugging tools for than that C has (Valgrind, GDB, IDEs, etc).

...makes your code harder to read:

by lessening its homogeneity
by making the user move around move between various source files
by lessening your IDE's ability to aid you in traversing the code, since some of the calls are embedded in strings as calls to an external
because Bash is often even harder to read than C

...makes your code harder to maintain, because of being harder to test and read.
...is unnecessary, because under the covers Bash is just calling the Unix C API anyway.
...is artistically distasteful— it smells like a kludge.
...adds a failure point to your code. Just look how many possible error messages you output, and then abort!
...presumably, this will also be a performance hit because of the overhead of running Bash.

I would strongly advise you not to use sys calls unless you are calling a very complex external application that would be difficult to reproduce in your C code in reasonable time and doesn't have a C API (and serious applications are not generally written in Bash) or you are performing an extremely low-level function that there is no C API for. Work for homogeneity—it's much cleaner.


Using Powershell to Copy Files to Several Locations This code copies files to our Test server and places the executable on a file share for the tester to access.  I'm new to Powershell, so I'm sure I'm not doing this in the most efficient way.  Any criticisms would be appreciated.

# the destination that we are copying TO.
$codeToDeploy = "\\deployment_server\App-Shares\Developer_Area\Code to Deploy\";

# all the local folders we'll need
$myProjectFolder = "C:\MTS\ProjectFolder\Project\";
$myLocalOurProjectName =  ($myProjectFolder + "OurProjectName\bin\Debug\");
$myLocalLauncher = ($myProjectFolder + "OurProjectName_Launcher\bin\Debug\");
$myLocalWcfService = ($myProjectFolder + "ProjectWCFService\bin\");
$myLocalWebsite = ($myProjectFolder + "MTSWebsite\bin\");

#----------------------------------------------------------------
# 1. prep the "code to deploy" folder on the server to receive files
#----------------------------------------------------------------

# set our current folder
Set-Location $codeToDeploy

# create the name of the new top level folder with today's date
$datedFolderName = "MM " + (get-date -Format "yyyy-MM-dd") + "\";

# create the date-named folder - overwrite if it already exists
New-Item $datedFolderName -ItemType directory -Force;

# set our current folder to the date-named folder
Set-Location $datedFolderName;

# create the sub folders
New-Item "OurProjectName Client" -ItemType directory -Force;
New-item "OurProjectName Launcher" -ItemType directory -Force;
New-item "ProjectWCFService\front end" -ItemType directory -Force;
New-item "Website\front end" -ItemType directory -Force;

#----------------------------------------------------------------
# 2. copy all the files from local debug to the target folders
#----------------------------------------------------------------

# copy to the MM Client folder
$destination = ($codeToDeploy + $datedFolderName + "OurProjectName Client\");
Set-Location $myLocalOurProjectName
Copy-Item *.dll,OurProjectName.exe $destination -Force;

# copy to the Launcher Folder
$destination = ($codeToDeploy + $datedFolderName + "OurProjectName Launcher\");
Set-Location $myLocalLauncher;
Copy-Item OurProjectName_Launcher.exe $destination -Force;

# copy to the Service Folder
$destination = ($codeToDeploy + $datedFolderName + "ProjectWCFService\");
Set-Location $myLocalWcfService;
Copy-Item *.dll $destination -Force;
Set-Location -Path ..
Copy-Item AllService.svc -Destination ($destination + "front end\") -Force;

# copy to the Website Folder
$destination = ($codeToDeploy + $datedFolderName + "Website\");
$frontEnd = ($destination + "front end\")
Set-Location $myLocalWebsite;
Copy-Item *.dll $destination -Force;
Set-Location -Path ..
Copy-Item *.aspx,*.master,CSS,SiteImages $frontEnd -Recurse -Force;

#----------------------------------------------------------------
# 3. copy all the files from local debug to the server's
#    "client install\binaries" location for serving up to the launcher
#----------------------------------------------------------------

# copy to the MM Client folder
$destination = "\\app-ttps2-tst\App-Shares\Project\Client Install\Binaries\OurProjectName\";
Set-Location $myLocalOurProjectName
Copy-Item *.dll,OurProjectName.exe -Destination $destination -Force;

#----------------------------------------------------------------
# 4. copy all the files from local debug to the UI Testing location
#----------------------------------------------------------------

$folders = Get-ChildItem "\\nw\data\TnFS\Project\ProjectFolder\UI TESTING\MM *" | Sort-Object

#save the latest config file for later
#$latestConfigFile = $folders | Select-Object -Last  | Get-ChildItem OurProjectName.exe.config

# copy to the MM Client folder
$destination = "\\nw\data\TnFS\Project\ProjectFolder\UI TESTING\" + $datedFolderName;
New-Item $destination -ItemType directory -Force;

Set-Location $myLocalOurProjectName
Copy-Item *.dll,OurProjectName.exe -Destination $destination -Force;

Simple program at first glance. I am glad to see some documentation there that explains what the code does. This is a good and important practice.

Set-Location

This is the only real problem I see with the code here. You rely on the working directory of PowerShell when copying files. You already know the directories you are using. Use the full paths and skip Set-Location altogether. This will reduce complexity and points of failure.

Instead of

# copy to the MM Client folder
$destination = ($codeToDeploy + $datedFolderName + "OurProjectName Client\");
Set-Location $myLocalOurProjectName
Copy-Item *.dll,OurProjectName.exe $destination -Force;

Use

# copy to the MM Client folder
$destination = ($codeToDeploy + $datedFolderName + "OurProjectName Client\");
Copy-Item -Path $myLocalOurProjectName -Include "*.dll","OurProjectName.exe" -Destination $destination -Force

In this same vein use the parameter names when writing code. Position arguments are fine for testing but readability improves for you and others when you use named arguments.

New-Item

When creating new folders PowerShell will output the Directory Object when it is created. Most find this undesirable. To void the output a clean approach is to pipe Out-Null. There are other ways to do this as well.

New-Item $destination -ItemType directory -Force | Out-Null

You might like the output but just beware it is there and being generated by New-Item.

String based path creation

This can work fine but a better practice is to use cmdlets and methods to build paths reliably. This way you don't have to worry about leading and trailing as much. When it comes to stitching more that 2 folder/paths together Combine is your friend.

Instead of

$destination = ($codeToDeploy + $datedFolderName + "OurProjectName Client\");

Consider

[io.path]::Combine($codeToDeploy, $datedFolderName, "OurProjectName Client\")

Code Repetition

This is largely made moot by what was said previously but keep this in mind. You have small code blocks that are repetitions but use the same logic with just some strings changing. Functions should be used here so that if some logic has to be added you are making the change in one place as supposed to updating several snippets throughout the code.

Pointless Code

Understand that I really wanted another word other than pointless. You have at least one line that is not serving a purpose

$folders = Get-ChildItem "\\nw\data\TnFS\Project\ProjectFolder\UI TESTING\MM *" | Sort-Object

There was commented code related to the above line but, since it was commented, the above line serves no purpose. Something to watch out for.


Generic error handler function for POSIX shell scripts Intention

I came with the idea of generic, portable, highly reliable, and further customizable function for Shell scripts, written in POSIX, for error handling.

Purpose

The function shall find out, if the terminal has color support, and act accordingly. If it has, then I highlight the error origin and the exit code with different colors. The main message shall be tabbed from the left, all for the best readability.

Example function call and error handler output (textual) + Explanation

print_usage_and_exit is a self-explanatory function, which watches over the number of given arguments, it accepts exactly one, let's give it some more:

Example function:

print_usage_and_exit()
{
    # check if exactly one argument has been passed
    [ "${#}" -eq 1 ] || print_error_and_exit 1 "print_usage_and_exit" "Exactly one argument has not been passed!\\n\\tPassed: ${*}"

    # check if the argument is a number
    is_number "${1}" || print_error_and_exit 1 "print_usage_and_exit" "The argument is not a number! Exit code expected."

    echo "Usage:   ${0} [-1]"
    echo "         -1: One-time coin collect."
    echo "Default: Repeat coin collecting until interrupted."
    exit "${1}"
}

Example function call - erroneous:

print_usage_and_exit a b c 1 2 3

Example output:

print_usage_and_exit()

        Exactly one argument has not been passed!
        Passed: a b c 1 2 3

exit code = 1

Example error handler output (visual)

The actual error handler function code

print_error_and_exit()
# expected arguments:
# $1 = exit code
# $2 = error origin (usually function name)
# $3 = error message
{
    # check if exactly 3 arguments have been passed
    # if not, print out an internal error without colors
    if [ "${#}" -ne 3 ]
    then
        printf "print_error_and_exit() internal error\\n\\n\\tWrong number of arguments has been passed: %b!\\n\\tExpected the following 3:\\n\\t\\t\$1 - exit code\\n\\t\\t\$2 - error origin\\n\\t\\t\$3 - error message\\n\\nexit code = 1\\n" "${#}" 1>&2
        exit 1
    fi

    # check if the first argument is a number
    # if not, print out an internal error without colors
    if ! [ "${1}" -eq "${1}" ] 2> /dev/null
    then
        printf "print_error_and_exit() internal error\\n\\n\\tThe first argument is not a number: %b!\\n\\tExpected an exit code from the script.\\n\\nexit code = 1\\n" "${1}" 1>&2
        exit 1
    fi

    # check if we have color support
    if [ -x /usr/bin/tput ] && tput setaf 1 > /dev/null 2>&1
    then
        # colors definitions
        readonly bold=$(tput bold)
        readonly red=$(tput setaf 1)
        readonly yellow=$(tput setaf 3)
        readonly nocolor=$(tput sgr0)
        # combinations to reduce the number of printf references
        readonly bold_red="${bold}${red}"
        readonly bold_yellow="${bold}${yellow}"
        # here we do have color support, so we highlight the error origin and the exit code
        printf "%b%b()\\n\\n\\t%b%b%b\\n\\nexit code = %b%b\\n" "${bold_yellow}" "${2}" "${nocolor}" "$3" "${bold_red}" "${1}" "${nocolor}" 1>&2
        exit "$1"
    else
        # here we do not have color support
        printf "%b()\\n\\n\\t%b\\n\\nexit code = %b\\n" "${2}" "${3}" "${1}" 1>&2
        exit "$1"
    fi
}

EDIT

I realized the tput could very well be anywhere else than in /usr/bin/.

So, while keeping the original code untouched, I have changed the line:

if [ -x /usr/bin/tput ] && tput setaf 1 > /dev/null 2>&1

to a more plausible check:

if command -v tput > /dev/null 2>&1 && tput setaf 1 > /dev/null 2>&1

I'm a big fan of tput, which many script authors seem to overlook, for generating appropriate terminal escapes.  Perhaps my enthusiasm started back in the early '90s, when using idiosyncratic (non-ANSI) terminals, but it still glows bright whenever I run a command in an Emacs buffer or output to a file.

A style point, on which you may disagree: I prefer not to enclose parameter names in braces unless it's required (for transforming the expansion, or to separate from an immediately subsequent word).  So [ "$#" -ne 3 ] rather than [ "${#}" -ne 3 ], for example.  The braces aren't wrong, but do feel unidiomatic there.

A simple improvement: you can save a lot of doubled-backslashes in the printf format strings by using single quotes rather than double quotes (this also protects you against accidentally expanding variables into the format string).  I also find it easier to match arguments to format if I make judicious use of line continuation.  Example:
    printf '%s%b()\n\n\t%s%b%s\n\nexit code = %b%s\n' \
           "$bold_yellow" "$2" \
           "$nocolor" "$3" "$bold_red" \
           "$1" "$nocolor" >&2

The definition of is_number isn't shown in your example function, but something similar is written in full here:
# check if the first argument is a number
# if not, print out an internal error without colors
if ! [ "${1}" -eq "${1}" ] 2> /dev/null
then

I think it makes sense to have is_number for that test; it would certainly reduce the need for comments:
if ! is_number "$1"
then    # print out an internal error without colors

It probably makes sense to redirect output to the error stream once at the beginning of print_error_and_exit, since we won't be generating any ordinary output from this point onwards:
print_error_and_exit()
{
    exec >&2

That saves the tedium of adding a redirect to every output command, and neatly avoids the easy mistake of missing one.

When we test that tput succeeds, perhaps we should tput sgr0 without redirecting to null, so that the output device is in a known state (thus killing two birds with one stone)?
If tput works at all, then when its argument can't be converted, its output is empty, which is fine.  If we want to be really robust when tput doesn't even exist, then we could test like this:
tput sgr0 2>/dev/null || alias tput=true

Then we don't need a separate branch for the non-coloured output (we'll just output empty strings in the formatting positions).  That won't quite work as I've written it, unless we specifically export the alias to sub-shells, but we can more conveniently just use a variable:
tput=tput
$tput sgr0 2>/dev/null || tput=true

We might also choose to test that the output is a tty (test -t 1, if we've done the exec I suggested, else test -t 2).

$bold_yellow saves no typing compared to $bold$yellow, and it's only used once anyway, so it can easily be eliminated.  Same for $bold$red.

Modified code
Applying my suggestions, we get:
is_number()
{
    test "$1" -eq "$1" 2>/dev/null
}

print_error_and_exit()
# expected arguments:
# $1 = exit code
# $2 = error origin (usually function name)
# $3 = error message
{
    # all output to error stream
    exec >&2

    if [ "$#" -ne 3 ]
    then    # wrong argument count - internal error
        printf 'print_error_and_exit() internal error\n\n\tWrong number of arguments has been passed: %b!\n\tExpected the following 3:\n\t\t\$1 - exit code\n\t\t\$2 - error origin\n\t\t\$3 - error message\n\nexit code = 1\n' \
               "$#"
        exit 1
    fi

    if ! is_number "$1"
    then    # wrong argument type - internal error
        printf 'print_error_and_exit() internal error\n\n\tThe first argument is not a number: %b!\n\tExpected an exit code from the script.\n\nexit code = 1\n' \
               "$1"
        exit 1
    fi

    # if tput doesn't work, then ignore
    tput=tput
    test -t 1 || tput=true
    $tput sgr0 2>/dev/null || tput=true

    # colors definitions
    readonly bold=$($tput bold)
    readonly red=$($tput setaf 1)
    readonly yellow=$($tput setaf 3)
    readonly nocolor=$($tput sgr0)

    # highlight the error origin and the exit code
    printf '%s%b()\n\n\t%s%b%s\n\nexit code = %b%s\n' \
           "${bold}$yellow" "$2" \
           "$nocolor" "$3" "${bold}$red" \
           "$1" "$nocolor"

    exit "$1"
}


Accumulate the charAt(*) for each n This is a helper function, along with three others. It should be called as a .filter(); to return only prime numbers. It is for one of the exercises at FreeCodeCamp.com, "Sum all Primes".

I'm just trying to accumulate the charAt(*) for each n, so 10 would return 1, being 1+0 21 would return 3, being 2+1 and 977 would return 23, being 9+7+7.

I've commented out some of my attempts before thinking of charAt(0) and some n's will have as many as 3 digits, maybe more if I don't have to limit it, but the largest test case stops at 997 I think, but an extra decimal for margin of error. I finally got something to work, but it feels like I'm going the long way to get there.. Surely there's something already built into to JavaScript I'm overlooking to reduce the value of a string. Or should I be trying to turn it into an array of digits?

function isItTHREED(n) {
  //  https://www.thoughtco.com/how-to-determine-number-is-prime-2312518

   //  Try 3.  Take the number, and add the digits up, when those digits are divisible by 3, the number is not prime.
  //          Take 2469, those digits add up to 21, and 21 is divisible by 3, therefore 2469 is not a prime number.
console.log('3s',n);
  n = n.toString();
  var digitONE = Number(n.charAt(0));
  var digitTWO = Number(n.charAt(1));
  var digitTHREE = Number(n.charAt(2));
  var digitFOUR = Number(n.charAt(3));
  console.log('digits',digitONE);
  console.log('digits',digitTWO);
  console.log('digits',digitTHREE);
  console.log('digits',digitFOUR);
  var testing = digitONE + digitTWO +digitTHREE + digitFOUR
  console.log(testing);
  //if(testing % 3 != 0)

  /*
  n = n.toString();
  n = n.split('');
  //num = num.split('');
  n = [n];
  console.log("num", n);
  */
   n = testing;
  return n;
} //end of isItTHREED

isItTHREED(10); // should return 1
isItTHREED(21); // should return 3
isItTHREED(977); // should return 23.

CodePen

As others have mentioned, you are attempting to find the digital sum of a given number. In JavaScript, you can do the following:

function digitalSum(number) {
  return (
    number
      .toString()
      .split('')
      .map(c => parseInt(c))
      .reduce((a, b) => a + b)
  )
}

As it intuitively appears, the code above does the following:

convert the number into a string
split the string by the empty string, thus each digit/character of the string is an individual element of the new list
converts each digit from a string to an actual number
Reduces the list by the sum function

There are other ways of accomplishing this task, but I believe the above is rather easy to understand.


Two similar shell functions that run different programs I have a script that contains two shell functions:

runhaskells() {
    local DIR=$PWD
    local TARGET="cabal.sandbox.config"
    while [[ ! -e $DIR/$TARGET -a $DIR != "/" ]]; do
        DIR=$(dirname $DIR)
    done
    if [[ $DIR != "/" ]]; then
        local DB=$(sed -ne '/^package-db: */{s///p;q;}' "$DIR/$TARGET")
        runhaskell -no-user-package-db -package-db="$DB" "$@"
    else
        runhaskell "$@"
    fi
}

ghcs() {
    local DIR=$PWD
    local TARGET="cabal.sandbox.config"
    while [[ ! -e $DIR/$TARGET -a $DIR != "/" ]]; do
        DIR=$(dirname $DIR)
    done
    if [[ $DIR != "/" ]]; then
        local DB=$(sed -ne '/^package-db: */{s///p;q;}' "$DIR/$TARGET")
        ghc -no-user-package-db -package-db="$DB" "$@"
    else
        ghc "$@"
    fi
}

which obviously is a repeat of each other except for the fact that it calls a different program in each function.  The first function calls the runhaskell program while the second function calls the ghc program.

How can I DRY this script (either zsh or bash syntax is fine for me) with the end goal that I should be able to call either function at the command line?

Two simple bash solutions I can think of are:

Use a shell script and $0/$BASH_SOURCE to determine the calling name and a symlink from one name to the other.
Two small wrapper functions.

h() {
    local DIR=$PWD
    local TARGET="cabal.sandbox.config"
    while [[ ! -e $DIR/$TARGET -a $DIR != "/" ]]; do
        DIR=$(dirname $DIR)
    done
    if [[ $DIR != "/" ]]; then
        local DB=$(sed -ne '/^package-db: */{s///p;q;}' "$DIR/$TARGET")
        set -- "${@:1:1}" -no-user-package-db -package-db="$DB" "${@:2}"
        # or this is the order of original arguments and added arguments don't matter
        set -- "$@" -no-user-package-db -package-db="$DB"
    fi

    "$@"
}

runhaskells() {
    h runhaskell "$@"
}

ghcs() {
    h ghc "$@"
}

Slightly DRYier version of the above which uses bash's FUNCNAME array.

    h() {
        local DIR=$PWD
        local TARGET="cabal.sandbox.config"
        while [[ ! -e $DIR/$TARGET -a $DIR != "/" ]]; do
            DIR=$(dirname $DIR)
        done
        if [[ $DIR != "/" ]]; then
            local DB=$(sed -ne '/^package-db: */{s///p;q;}' "$DIR/$TARGET")
            set -- "${@:1:1}" -no-user-package-db -package-db="$DB" "${@:2}"
            # or this is the order of original arguments and added arguments don't matter
            set -- "$@" -no-user-package-db -package-db="$DB"
        fi

        "${FUNCNAME[1]%s}" "$@"
    }

    runhaskells() {
        h "$@"
    }

    ghcs() {
        h "$@"
    }


Printing odd and even numbers with two threads I would like to refactor this code printing odd and even numbers with two threads considering agile practice:

public class EvenOddNumberPrinter {

    private static class NumberPrinter {

        // To check if even number is printed or not.
        private boolean isEvenPrinted = true;

        public void printOdd(int number) throws InterruptedException {
            // Get a lock on NumberPrinter
            synchronized (this) {
                // Wait until even is not printed.
                if (!isEvenPrinted)
                    wait();

                System.out.println(number);
                isEvenPrinted = false;

                // Notify the other waiting thread which is waiting on
                // NumberPrinter
                // Other thread will get out of waiting state
                notify();
            }
        }

        public void printEven(int number) throws InterruptedException {
            synchronized (this) {
                if (isEvenPrinted)
                    wait();

                System.out.println(number);
                isEvenPrinted = true;
                notify();
            }
        }
    }

    private static class OddNumberGenerator implements Runnable {

        private NumberPrinter q;
        private int max;

        public OddNumberGenerator(NumberPrinter q, int max) {
            this.q = q;
            this.max = max;
        }

        @Override
        public void run() {
            for (int i = 1; i < max; i = i + 2) {
                try {
                    q.printOdd(i);
                } catch (InterruptedException ex) {
                    ex.printStackTrace();
                }
            }
        }
    }

    private static class EvenNumberGenerator implements Runnable {

        private NumberPrinter printer;
        private int max;

        public EvenNumberGenerator(NumberPrinter printer, int max) {
            this.printer = printer;
            this.max = max;
        }

        @Override
        public void run() {
            for (int i = 2; i <= max; i = i + 2) {
                try {
                    printer.printEven(i);
                } catch (InterruptedException ex) {
                    ex.printStackTrace();
                }
            }
        }
    }

    public static void main(String[] args) {
        int maxNumber = 10;
        NumberPrinter printer = new NumberPrinter();
        new Thread(new EvenNumberGenerator(printer, maxNumber)).start();
        new Thread(new OddNumberGenerator(printer, maxNumber)).start();
    }

}

Inconsistency
In OddNumberGenerator your printer is called q.  In EvenNumberGenerator it is called printer.  Why the difference?
The odd one prints to max-1 but the even one prints to max.  They should both print to max, otherwise you are assuming that max must be even.
No check after wait
After your wait() call, you don't check to see if you are in the correct state before printing the next number.  If both threads called printOdd(), it would happily print two or more odd numbers in a row.  It's not a problem with your program because your two threads are well behaved, but it isn't a very good example of using synchronization.
Simplification
You don't need separate printOdd() and printEven() functions that do almost the same thing.  You could combine them into a single printNumber() function.
Similarly, you don't need two number generators.  If you just added a start variable to the number generator, you could create an odd generator with a start value of 1 and an even generator with a start value of 2.


My Javascript form validation Hello today after few weeks of learning I tried some form validation.
I am wondering how I could improve my code and what things I missed.
Here is a preview hosted on github + repository.
And here is my code :
HTML
<!DOCTYPE html>

    <meta charset="utf-8">
    <link rel="stylesheet" href="main.css">
    <link href="https://fonts.googleapis.com/css2?family=Rubik:wght@300;400;500;600;700&display=swap" rel="stylesheet">
    <title>Form validation</title>

    <form action="#" class="form">

Contact with us

          <input type="text" name="name" class='input-name' id='input' placeholder="First name *" required>

          <input type="text" name="surname" class='input-surname' id='input' placeholder="Surname">

          <input type="text" name="email" class='input-email' id='input' placeholder="E-mail *" required>

          <input type="text" name="phone" class='input-phone' id='input' placeholder="Phone number">

        <textarea name="message" rows="8" cols="80" class='input-message' id='input' placeholder="Message *" required></textarea>
        <button href="#" class="btn">Submit</button>

    </form>

JS
'use strict';

const submit = document.querySelector('.btn');
const name = document.querySelector('.input-name');
const surname = document.querySelector('.input-surname');
const email = document.querySelector('.input-email');
const phone = document.querySelector('.input-phone');
const items = document.querySelectorAll('#input');

const isValid = function(item) {
  for (let i = 0; i < item.length; i++) {
    if (item[i].value){ // Check if contains value

      let error = document.getElementById(`${item[i].className}-p`);
      const letters = /^[A-Za-z]+$/;
      const numbers = /^\d+$/;
      let inputLength = item[i].value.length;

      switch (item[i].className){
        case 'input-name':
        case 'input-surname':
          if(!letters.test(item[i].value)){ // Invalid input
            error.textContent = 'Invalid data';
            error.classList.remove('p-hidden');
          }else
          error.classList.add('p-hidden');
          break;

        case 'input-email':
          if(!item[i].value.includes('.') || !item[i].value.includes('@') || !letters.test(item[i].value[inputLength-1])  ){ // check if includes (@ or .) and check if the last index is a letter
            error.textContent = 'Invalid data';
            error.classList.remove('p-hidden');
          }else
          error.classList.add('p-hidden');
          break;
        case 'input-phone':
          if(!numbers.test(item[i].value) ||  inputLength<5){  // Invalid input
            error.textContent = 'Invalid data';
            error.classList.remove('p-hidden');
          }else
          error.classList.add('p-hidden');
          break;
      }

    }
  }
}

for (let i = 0; i < items.length; i++) {
    items[i].addEventListener('click',function(){
      // Check if the input is valid
      isValid(items);
    });
}

Duplicate IDs are invalid HTML You have multiple elements of id='input', which is not permitted in HTML. If multiple elements need a particular attribute, use classes instead. IDs should be reserved for elements that are going to be absolutely unique on a page (or, you could also consider not using IDs at all, since they implicitly create global variables, which can lead to hard-to-understand bugs).
Input iteration You put the input collection into a variable named items, which is good: const items = document.querySelectorAll('#input'); but then you pass the collection to a function whose parameter is named item, and you do:
for (let i = 0; i < item.length; i++) {
  if (item[i].value) { // Check if contains value
    // a long block
  }

A reader of the code wouldn't expect an individual item to have a length and a numeric index. How about calling the parameter items instead - or, even better, inputs?

Rather than iterate over the indicies of each element, since you don't actually care about the indicies, but only about the underlying elements, it might be preferable to use for..of instead, so you never have to reference the indicies.

Nested indentation can be difficult to read. Instead of a long block inside an if statement, consider continuing the loop early instead:

for (const input of inputs) {
  if (!input.value) {
    continue;
  }
  // put validation code here

Or put it into a function:
for (const input of inputs) {
  if (input.value) {
    validateInput(input);
  }

Check validity on blur, not on click. Your current implementation will show errors only after the user has inputted something invalid, focused away, then clicked on the input box again. Better to inform them immediately, as soon as a box is de-focused.
Check validity even if input is empty, since errors may be displayed - if the input is empty, you'll want to clear the error. You could also consider clearing errors when an input is focused (so that errors are only displayed when an input currently isn't active).
Error text is always the same, so don't set it via the JS - put it into the HTML, and hide the error on pageload.
Use the case-insensitive flag in regular expressions instead of repeating both the capital and lowercase versions, eg: /^[a-z]$/i
Wording Contact with us would be better as Contact us
DRY input navigation There are a few sources of repetitiveness in the code:

Separate class names for each input, requiring iteration through each class name in the switch
Separate error class names for each input
Separate logic for each class name

You can make this better by:

Use an object indexed by the name attribute of each input, whose values are regular expressions to test the values against
Navigate to the input's adjacent error element with nextElementSibling instead of having separate error classes

For the email, you can use a regular expression so that the validation is of the same shape as for the other inputs.
const validators = {
  name: /^[a-z]$/i,
  surname: /^[a-z]$/i,
  email: /^(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])$/,
  phone: /^\d{5,}$/,
};
for (const input of document.querySelectorAll('#input')) {
  input.addEventListener('blur', () => {
    checkValidity(input);
  });
}
const checkValidity = (input) => {
  const isBad = validators[input.name].test(input.value);
  input.nextElementSibling.classList.toggle('p-hidden', isBad);
};

That's all you need.

'use strict';

const validators = {
  name: /^[a-z]+$/i,
  surname: /^[a-z]+$/i,
  email: /^(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])$/,
  phone: /^\d{5,}$/,
};
for (const input of document.querySelectorAll('.wrapper input')) {
  input.addEventListener('blur', () => {
    checkValidity(input);
  });
}
const checkValidity = (input) => {
  const isBad = validators[input.name].test(input.value);
  input.nextElementSibling.classList.toggle('p-hidden', isBad);
};
.p-hidden {
  display: none;
}
<form action="#" class="form">

Contact us

      <input name="name" placeholder="First name *" required>

Invalid data

      <input name="surname" placeholder="Surname">

Invalid data

      <input name="email" placeholder="E-mail *" required>

Invalid data

      <input name="phone" placeholder="Phone number">

Invalid data

    <textarea name="message" rows="8" cols="80" placeholder="Message *" required></textarea>
    <button href="#" class="btn">Submit</button>

</form>

The email regex is probably more complicated than it needs to be, you could write a much more easy-to-read one with only a handful of characters that works just as well in 99% of situations, if you wanted.
Another option would be to remove all the JavaScript, and use the pattern attribute and type="email" in the HTML instead, letting the browser inform the user of invalid inputs:

<form action="#" class="form">

Contact us

      <input name="name" placeholder="First name *" required pattern="[a-zA-z]+">

      <input name="surname" placeholder="Surname" pattern="[a-zA-z]+">

      <input name="email" placeholder="E-mail *" required type="email">

      <input name="phone" placeholder="Phone number" pattern="\d+{5,}">

    <textarea name="message" rows="8" cols="80" placeholder="Message *" required></textarea>
    <button href="#" class="btn">Submit</button>

</form>

Result:


Better way to repeatedly use istringstream in "counting minutes" challenge I was doing this easy challenge "Counting Minutes I" from Coderbyte.com where you just calculate the number of minutes between two given times.

The second time is always after the first, although it may look to be before but mean it's during the next day.

The format of their test cases is "hour1:minute1-hour2:minute2" such as "12:30am-12:00pm" where the difference would be 11.5 hours = 1425 minutes.

In converting different parts of the string containing the times to ints I've been using istringstream but creating a new one each time I call the function to set the ints. It seems like this is a sloppy way of accomplishing this, is there a better way like creating one istringstream in my main CountingMinutesI function and passing that as an argument to getNum then clearing it out after setting each particular int? Or is this also a bad idea?

#include <iostream>
#include <sstream>
using namespace std;

void getNum (string timeString, int& pos, int& result) {
  string temp;
  while (isdigit(timeString[pos])) {
    temp += timeString[pos];
    pos++;
  }
  istringstream iss (temp);
  iss >> result;
}

void to24HourTime(string timeString, int& pos, int& hourToConvert) {
  if (timeString[pos] == 'p' && hourToConvert != 12)
    hourToConvert += 12;
  if (timeString[pos] == 'a' && hourToConvert == 12)
    hourToConvert = 0;
}

int CountingMinutesI(string str) {

  int hour1int, hour2int, min1int, min2int, minDifference;
  int stringPos = 0;

  getNum(str, stringPos, hour1int);
  stringPos++;
  getNum(str, stringPos, min1int);

  to24HourTime(str, stringPos, hour1int);

  stringPos += 3;

  getNum(str, stringPos, hour2int);
  stringPos++;
  getNum(str, stringPos, min2int);

  to24HourTime(str, stringPos, hour2int);

  if (hour2int < hour1int) //second time is the during the next day
    hour2int += 24;
  if (hour1int == hour2int && min1int > min2int) //time difference would look negative, but is really between 23 and 24 hours
    hour2int += 24;
  if (min2int < min1int) { //simplify subtracting minutes to add to total difference
    min2int += 60;
    hour2int--;
  }

  minDifference = (hour2int - hour1int) * 60 + (min2int - min1int);

  return minDifference;

}

int main() {

  // keep this function call here
  cout << CountingMinutesI(gets(stdin));
  return 0;

}

In to24HourTime(), pos doesn't need to be passed by reference as it's not being modified.  Just pass it by value.

Instead of passing hourToConvert by reference (which is being modified), you can just pass by value and then return the result.  It may also increase readability as it'll be clear that the third argument is to be reassigned by the function.

Final changes:

hour1int = to24HourTime(str, stringPos, hour1int);

// ...

int to24HourTime(string timeString, int pos, int hourToConvert) {
    if (timeString[pos] == 'p' && hourToConvert != 12)
        hourToConvert += 12;
    if (timeString[pos] == 'a' && hourToConvert == 12)
        hourToConvert = 0;

    return hourToConvert;
}