Hypotenuse and name generator I am a self-taught programmer and I just started learning Java and wrote a simple program. If anyone more experienced than myself can go over my code and give me some pointers I would really appreciate it.

The program starts by asking the user to enter either 0 for the name generator or 1 for the hypotenuse generator (uses the Pythagorean Theorem). Then it asks for follow up questions based on the starting choice. (I'm using Terminal to run this program.)

Simple.java

import java.util.Scanner;
public class Simple {
    static int a = 13;
    static int b = 27;
    public static void main(String args[]) {
        Scanner reader1 = new Scanner(System.in);
        System.out.println("Name Gen = 0, Hypotenuse Calc = 1");
        int choice = reader1.nextInt();

        if (choice == 1) {
            System.out.println("enter num for a");
            a = reader1.nextInt();
            System.out.println("enter num for b");
            b = reader1.nextInt();
            SimpleTwo.firstMethod(a, b);
            reader1.close();
            String[] arg = new String[0];
        }else{
            SimpleTwo obj = new SimpleTwo();
            System.out.println("age?");
            int n = reader1.nextInt();
            obj.age = n;
            System.out.println("first name?");
            String fname = reader1.next();
            obj.fName = fname;
            System.out.println("last name?");
            String lname = reader1.next();
            obj.lName = lname;
            reader1.close();
            SimpleTwo.myMethod();
        }

    }
}

SimpleTwo.java

public class SimpleTwo {
    static int age = 0;
    static String fName = "";
    static String lName = "";

    public static void myMethod() {
        System.out.println(age+", "+fName+", " + lName);
    }

    public static void firstMethod(int a, int b) {
        int ab = (a * a) + (b * b);
        double c = Math.sqrt(ab);
        System.out.println("The length of the Hypotenuse is " + c + ", when a=" + a + " & b=" + b + ".");
    }
}

SimpleTwo obj = new SimpleTwo();

is unnecessary because all your properties and methods of SimpleTwo are static, therefore creating an instance of SimpleTwo does nothing unless you have something in it that's not static. I would also guess that lines with code like obj.age give you a compiler warning because you should be using SimpleTwo.age since age is a static member.

I would also change the names of the classes and the methods to something more meaningful. firstMethod doesn't describe what it is for and technically it is the second method in your class


Checking consecutive characters in a string First of all, this code will ask for a number (first) on how many test the user would like to have. Then the user will input the strings depending on how the number entered.

Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
    Dim input As String = txtInput.Text
    Dim inputArr As String() = input.Split(New Char() {Environment.NewLine})
    Dim count As Integer = Convert.ToInt32(inputArr(0))
    If count >= 1 And count <= 10 Then
        If txtInput.Lines.Length() - 1 <= count Then
            For x As Integer = 1 To count
                If inputArr(x).Length - 1 >= 1 And inputArr(x).Length - 1 <= 10 Then
                    Dim numDeletes = CountConsecutiveDuplicates(inputArr(x))
                    txtResult.AppendText(numDeletes & Environment.NewLine)
                Else
                    MessageBox.Show("Hell no!")
                End If
            Next x
        Else
            MessageBox.Show("Max test reached!")
        End If

    Else
        MessageBox.Show("Number of cases is limited only from 1 - 10")
    End If
End Sub

Shared Function CountConsecutiveDuplicates(Of T)(input As IEnumerable(Of T)) As Int32
    Dim count As Int32 = 0
    Dim comparer = EqualityComparer(Of T).Default

    ' optimization for lists and arrays: '
    Dim listT = TryCast(input, IList(Of T))
    If listT IsNot Nothing Then
        If listT.Count <= 1 Then Return 0
        For i As Int32 = 0 To listT.Count - 2
            If comparer.Equals(listT(i), listT(i + 1)) Then
                count += 1
            End If
        Next
        Return count
    End If

    If Not input.Any() Then Return 0
    Dim this As T = input.First()
    For Each item As T In input.Skip(1)
        If comparer.Equals(this, item) Then
            count += 1
        End If
        this = item
    Next
    Return count
End Function

Here are the constraints:

tests >= 1 and tests <= 10
charofstring >= 1 and charofstring <= 10

This code will check the string that user input in a multiline textbox. For example, the input is 4 (as the test counter)(\n) AABBCCC (first string to check)(\n) AAAABBBC (2nd string to check)(\n) ... (4th string)

I need to check on the number of consecutive characters that are needed to delete until no characters are consecutive. This will be the result of the user input.

4 (first string because 1A 1B and 2C) - text in the parentheses not included in output
5 (2nd string because 3A 2B)
...

Is there any code that I do not need? Someone told me that I can still shorten the code I have. How can I modify this code?

"txtInput = name of input textbox"
"txtResult = name of output textbox"
"Button1 = name of button to execute the code."

By using the Distinct method and comparing lengths you can get the number you want in one line.  Here's what such a function could look like:

Function CharsToDelete(Of T)(input As IEnumerable(Of T)) As Integer
    Return input.Count - input.Distinct.Count
End Function

Oops missed the part about consecutive chars.  This should do the trick:

Function CharsToDelete(Of T)(input As IEnumerable(Of T)) As Integer
    Return input.Where(Function(x, y) x.Equals(input(y + 1))).Count
End Function

You don't need to split the text into an array.  The Lines property of the TextBox already does this:

Dim inputArr As String() = txtInput.Lines


MinutesPerDay - Does .NET have anything for it? Looking at a code review at work for a Win32 C# code I am seeing some lines like this:

int MinutesPerDay = 1440;
int MinutesPerWeek = 10080;
int MinutesPerMonth = 43200;

and so on ...

I am not sure what is wrong with it but still I feel this is not a good code and design? What do you guys think? Could he do it in a better way?

Since these are constants, I would declare them as such. Also, making the calculation explicit would be more informative.

private const int MinutesPerDay = 24 * 60;
private const int MinutesPerWeek = 7 * MinutesPerDay;
private const int MinutesPerMonth = 30 * MinutesPerDay;


Merging JSON objects with common values I am trying to merge JSON objects which have common values.

This is the original JSON:

{
    "setA":[
        {
            "setId":"setA"
            ,"prodId":"A"
            ,"price":"5"
            ,"delivertCost":"1"
            ,"name":"Set_Prod01"
        }

        ,{
            "setId":"setA"
            ,"prodId":"B"
            ,"price":"5"
            ,"delivertCost":"1"
            ,"name":"Set_Prod01"
        }
    ]
}

Since it has same setId and name, I want to make it like this:

{
    "setA": {
        "setId":"setA"
        ,"prodId":"A,B"
        ,"price":"5"
        ,"deliveryCost":3
        ,"name":"Set_Prod01"
    }
}

To do this, I made this JavaScript code, but it looks terrible and more elements have attributes with if else than needed.

function groupBy(coll, f) {
  return coll.reduce(function(acc, x) {
    var k = f(x);
    acc[k] = (acc[k] || []).concat(x);
    return acc;
  }, {});
}

var test = {
  items: [{
    setId: 'setA', prodId: "A", price: '5', deliveryCost : '1', name: "Set_Prod01"
  }, {
    setId: 'setA', prodId: "B", price :'5', deliveryCost : '2', name: "Set_Prod01"
  }]
};

console.log(JSON.stringify(test));

var result = groupBy(test.items, function(x){return x.setId});
//first grouping
console.log(JSON.stringify(result));

//begin spagetti code... T^T
var secondResult = {};
for(var key in result){
    var secondObjForSet = {};
    var setGroup = result[key];
    for(var i = 0; i < setGroup.length; i++) {
        var singleSetItem = setGroup[i];
        for(var innerKey in singleSetItem){
            if(innerKey == 'deliveryCost'){
                secondObjForSet[innerKey] = (typeof secondObjForSet[innerKey] === 'undefined') ? 0 + singleSetItem[innerKey] : (secondObjForSet[innerKey] * 1) + (singleSetItem[innerKey] * 1);
            }else if(innerKey == 'prodId'){
                secondObjForSet[innerKey] =
                    (typeof secondObjForSet[innerKey] === 'undefined' || secondObjForSet[innerKey] == '') ?
                        singleSetItem[innerKey] : secondObjForSet[innerKey] + ',' + singleSetItem[innerKey];
            }
            else{
                secondObjForSet[innerKey] = singleSetItem[innerKey];
            }
        }
    }
    secondResult[key] = secondObjForSet;
}

console.log(JSON.stringify(secondResult));

This is not flexible at all, so I need some help to make it more efficient.

I am assuming you have the correct target to transform (the input array is filtered).

If you are using es6:

const input = [
    {
        "setId": "setA",
        "prodId": "A",
        "price": 5,
        "deliveryCost": 1,
        "name": "Set_Prod01"
    },
    {
        "setId": "setA",
        "prodId": "B",
        "price": 5,
        "deliveryCost": 2,
        "name": "Set_Prod01"
    }
];

const output = {
    "setId": "setA",
    "prodId": "A,B",
    "price": 5,
    "deliveryCost": 3,
    "name": "Set_Prod01"
};

const groupBy = (input) => (
    Object.assign(
        input[0],
        {
            prodId: input.map(set => set.prodId).join(','),
            deliveryCost: input.map(set => set.deliveryCost).reduce( (cur, next) => cur + next)
        }
    )
);

console.log(output);
console.log(groupBy(input));

If you are using es6 and stage-2 object spread operator, change the function to:

const groupBy = (input) => (
    {
        ...input[0],
        prodId: input.map(set => set.prodId).join(','),
        delivertCost: input.map(set => set.deliveryCost).reduce( (cur, next) => cur + next)
    }
);

If you are not using es6, simplest method is use it, and use babel to convert it into es5 compatible code. It's less code, more readable and easier to maintain.

EDIT:
The function above simply take an array of objects and assign them into another object. Note that in Object.assign, the latter's value will overwrite the former's value with the same key, so you can easily implement the overwriting rule you mentioned. The value are simply some functional programming, one join the array of string, and the other sum up the values.


Complex number class in Lua This is my first attemp with Lua.
I decided to create this class, because it have lots of "tweaks" and "tricks".
I want to know if I did it as best practices Lua ways.
My Lua version is 5.4.1
require "math"

Complex = {
    IMAGINARY_CHAR = "i"
}

function Complex:new(r, i)
    o = {
        r = r,
        i = i
    }

    setmetatable(o, self)

    self.__index = self

    return o
end

function Complex:__newindex(r, i)
end

function Complex:__unm()
    return Complex:new(
        -self.r,
        -self.i
    )
end

function Complex:__add(other)
    if type(other) == "number" then
        return Complex:new(
            self.r + other,
            self.i
        )
    else
        return Complex:new(
            self.r + other.r,
            self.i + other.i
        )
    end
end

function Complex:__sub(other)
    return self:__add(- other);
end

function Complex:__mul(other)
    if type(other) == "number" then
        return Complex:new(
            self.r * other,
            self.i * other
        )
    else
        return Complex:new(
            self.r * other.r - self.i * other.i,
            self.i * other.r + self.r * other.i
        )
    end
end

function Complex:__eq(other)
    return
        self.r == other.r and
        self.i == other.i
end

function Complex:__lt(other)
    -- incorrect but for sorting
    if self.r == other.r then
        return self.i < other.i
    else
        return self.r < other.r
    end
end

function Complex:tostring(i)
    if type(i) ~= "string" then
        i = "i"
    end

    if self.r == 0 and self.i == 0 then
        return "( 0 )"
    elseif self.i == 0 then
        return "( " .. self.r .. " )"
--  elseif self.r == 0 then
--      return "( " .. i .. self.i .. " )"
    else
        return "( " .. self.r .. " + " .. i .. self.i .. " )"
    end
end

function Complex:__tostring(i)
    return self:tostring(self.IMAGINARY_CHAR)
end

function Complex:abs2()
    return self.r * self.r + self.i * self.i
end

function Complex:abs()
    return math.sqrt(self:abs2())
end

-- ===================================================================

Complex.IMAGINARY_CHAR = "j" -- Electrical notation

x = Complex:new(0, 10)
y = Complex:new(-5, 5)

print(x)
print(y)

print(x:abs())
print(y:abs())

print(x + y)
print(x - y)

print(x + 12)
print(x - 12)

print(x == y)
print(x ~= y)

print(x ~= x)
print(x ~= x)

print(x < y)
print(x > y)

print(x <= y)
print(x >= y)

Possible improvements:

localise functions of the math library, and sometimes, self.r and self.i. It will reduce the number of table lookups, improving the performance, and will make the expressions simpler,
define __call metamethod for Complex. It will allow you to replace Complex:new(x, y) with Complex(x, y),
the 'constructor' should correctly handle cases when it is called with one parameter (Complex(5) = 5 + 0i). It can be achieved with a 'nullsafe operator' emulated as local a = user_input or value_if_null,
in can also be mage more concise, using the fact that setmetatable returns the table,
__newindex metamethod is not needed here, unless you want to set the absolute value or phase with code like c.abs = 1 or code.arg = pi. In addition, the arguments to __newindex are the table, the absent key and value set to it,
you can emulate the ternary operator in Lua with and and or, which allows to write more concise code: local a = condition and on_success or on_failure,
your arithmetic metamethods should handle the cases when the first argument (self) is a number,
you can define conjugate method and __div metamethod for division,
to square a table element, I'd rather recommend using ^ rather than multiplying by self. It will save table lookups,
you can define methods for polar coordinates and __pow metamethod for Euler's and de Moivre's formulae,
the __lt metamethod can be simplified using boolean operations,
Complex:tostring() can be simplified using 'ternary' operators, also handling of negative imaginary parts can be improved,
whether to put the i before or after the imaginary part and whether to surround a complex number with parentheses can be customisable,
the expression tests can be automated somewhat with load() function.

The improved code:
local math = require 'math'
local pi, sin, cos, atan2 = math.pi, math.sin, math.cos, math.atan2
local sqrt, exp, ln = math.sqrt, math.exp, math.log
local floor, abs = math.floor, math.abs

Complex = {
    IMAGINARY_CHAR = 'i'
}

function Complex:new(r, i)
    self.__index = self
    return setmetatable({r = r, i = i or 0}, self)
end

setmetatable(Complex, {
    __call = function (tbl, r, i)
        return Complex:new(r, i)
    end}
)

function Complex:__unm()
    return Complex(-self.r, -self.i)
end

function Complex:__add(other)
    return type(self) == 'number'   and Complex(self + other.r, other.i)
        or type(other) == 'number'  and Complex(self.r + other, self.i)
                                     or Complex(self.r + other.r, self.i + other.i)
end

function Complex:__sub(other)
    return self + (-other);
end

function Complex:__mul(other)
    return type(self) == 'number'   and Complex(self * other.r, self * other.i)
        or type(other) == 'number'  and Complex(self.r * other, self.i * other)
                                     or Complex(self.r * other.r - self.i * other.i, self.i * other.r + self.r * other.i)
end

function Complex:conjugate()
    return Complex(self.r, -self.i)
end

function Complex:__div(denominator)
    -- https://www.mesacc.edu/~scotz47781/mat120/notes/complex/dividing/dividing_complex.html
    local conjugate = type(denominator) == 'number' and Complex(denominator) or denominator:conjugate()
    local new_numerator, new_denominator = self * conjugate, denominator * conjugate
    -- new_denominator is real.
    return Complex(new_numerator.r / new_denominator.r, new_numerator.i / new_denominator.r)
end

function Complex:abs2()
    return self.r ^ 2 + self.i ^ 2
end

function Complex:abs()
    return sqrt(self:abs2())
end

function Complex:polar(abs, arg)
    return Complex(abs * cos(arg), abs * sin(arg))
end

function Complex:arg()
    return atan2(self.i, self.r)
end

function Complex:exp()
    local abs = exp(self.r)
    return Complex(abs * cos(self.i), abs * sin(self.i))
end

function Complex:__pow(power)
    -- Euler:
    local x = type(self) == 'number' and self or self.i == 0 and self.r > 0 and self.r or nil
    if x then
        return (power * ln(x)):exp()
    else
    -- de Moivre:
        local n = type(power) == 'number' and power or power.i == 0 and power.r or nil
        if n and floor(n) == n then
            local abs, arg = self:abs(), self:arg()
            return Complex:polar(abs ^ n, n * arg)
        end
    end
end

function Complex:__eq(other)
    return
        self.r == other.r and
        self.i == other.i
end

function Complex:__lt(other)
    -- incorrect but for sorting:
    return self.r < other.r or self.r == other.r and self.i < other.i
end

function Complex:tostring(i, prefix, parentheses)
    local im = type(i) == 'string' and i or 'i'
    local r, i = self.r, self.i

    local str = ((r ~= 0 or i == 0) and r or '')
             .. (r ~= 0 and i > 0 and ' + ' or '')
             .. (i < 0 and ' - ' or '')
             .. (i ~= 0 and (prefix and im .. abs(i) or abs(i) .. im) or '')
    if parentheses then
        str = '(' .. str .. ')'
    end
    return str
end

function Complex:__tostring(i)
    return self:tostring(self.IMAGINARY_CHAR, self.PREFIX, self.PARENTHESES)
end

-- ===================================================================

Complex.IMAGINARY_CHAR = 'j' -- electrical notation.
Complex.PREFIX = true
Complex.PARENTHESES = true

local context = {
    Complex = Complex,
    x = Complex(0, 10),
    y = Complex(-5, 5),
    z = Complex(5),
    pi = pi
}

local cases = {
    'x', 'y', 'z', 'x:abs()',
    'x + y', 'x - y', 'x + 12', 'x - 12', '12 + x', '12 - x',
    'x * y', 'y * 12', '2 * x',
    'y:conjugate()', 'x / y', 'y / y', 'x / 5', '5 / x', '5 / x * x',
    'y:abs()', 'y:arg() / pi * 180', 'Complex:polar(2, pi / 4)', 'y:exp()', 'x ^ 2', '2 ^ x',
    'x == y', 'x ~= y', 'x < y', 'x > y', 'x <= y', 'x >= y'
}
for _, expr in ipairs(cases) do
    print(expr, assert(load('return ' .. expr, nil, 't', context))())
end
```


Checking dictionary for specified key The first part of code is for me very unreadable. I tried to write in another way but I'm open to your suggestion!

return ViewData.ModelState.FirstOrDefault(x => x.Key.Equals(parameterName)).Value != null && ViewData.ModelState.FirstOrDefault(x => x.Key.Equals(parameterName)).Value.Errors.Any(x => !string.IsNullOrEmpty(x.ErrorMessage)) ? "" : " ";

//Looks in a dictionary for a key called parameterName
    ModelState modelState = ViewData.ModelState.FirstOrDefault(x => x.Key.Equals(parameterName)).Value;
    //If I could find it I look for any error associatd to that and I return it as a single string
    return modelState != null && modelState.Errors.Any(x => !string.IsNullOrEmpty(x.ErrorMessage)) ? "" : " ";

Not only is this difficult to read, it's also inefficient.

Cache your duplicate LINQ lookup to save time and make it more readable.

As for using the ternary operator, that's up to you, but I prefer to use a full if statement when the line gets too long.

Additionally swap out "" for String.Empty, it shows your intent better, and also makes it more strongly contrast against " "

var state =ViewData.ModelState.FirstOrDefault(x => x.Key.Equals(parameterName));

if(state.Value != null &&
    state.Value.Errors.Any(x => !string.IsNullOrEmpty(x.ErrorMessage)))
{
    return string.Empty;
}
else
{
    return " ";
}

I'm assuming this next bit executes elsewhere, because the two return statements above would otherwise make this unreachable code.

Use var when the type is obvious from the right hand side of the assignment (although in this case it's a little debatable, I'd personally use var here because the variable name mimics the type).

You have a typo in your second comment.

//Looks in a dictionary for a key called parameterName
var modelState = ViewData.ModelState.FirstOrDefault(x => x.Key.Equals(parameterName)).Value;
//If I could find it I look for any error associated to that and I return it as a single string
if(modelState.Value != null &&
    modelState.Value.Errors.Any(x => !string.IsNullOrEmpty(x.ErrorMessage)))
{
    return string.Empty;
}
    else
{
    return " ";
}

Finally, structure-wise I'd recommend converting the if-statement above into a separate method, since you call it in multiple places. This will leave a single point to modify during refactoring.

private string ConvertModelStateErrorsToString(ModelState modelState)
{
    if(modelState.Value != null &&
        modelState.Value.Errors.Any(x => !string.IsNullOrEmpty(x.ErrorMessage)))
    {
        return string.Empty;
    }
        else
    {
        return " ";
    }
}

Leaving your other code samples as:

var state =ViewData.ModelState.FirstOrDefault(x => x.Key.Equals(parameterName));

return ConvertModelStateErrorsToString(state);

And the other sample:

//Looks in a dictionary for a key called parameterName
var modelState = ViewData.ModelState.FirstOrDefault(x => x.Key.Equals(parameterName)).Value;
//If I could find it I look for any error associated to that and I return it as a single string
return ConvertModelStateErrorsToString(modelState);


Error handling data from XML document I am getting information from an XML document and for the most part everything runs as it should. Occasionally, I get a php fatal error Call to a member function children() on a non-object. I have tried to use this

if(isset($current->AttributeSets->children('ns2', true)->ItemAttributes->ListPrice->Amount))
{
    $listPrice = $current->AttributeSets->children('ns2', true)->ItemAttributes->ListPrice->Amount;
} else {
    $listPrice = 0;
}

to catch any times there is no information in $current, but it does not always work. How do I handle the errors so it works all the time?

The trouble is that your check can fail.  Before you can call the children method you need to be sure that you are dealing with an object (that provides the children method).  This is actually what your fatal error is telling you ($current->AttributeSets is a non-object).  You get this in your if statement within your isset call.

The trouble with fatal errors is that they are fatal.  There is no way to recover from them.  The PHP engine is about to shutdown.  You can read more about handling errors in an answer I wrote here.  The only way is to avoid fatal errors.  Don't call functions that don't exist, or methods on something that is NULL.

Possibly the reason you have this problem is breaking the Law of Demeter.  I don't know how the XML objects you are working with are built, etc.

You could solve it by checking each component.

Unfortunately you can't do this because isset can't use a function's return value:

// THIS WON'T WORK
isset($current->AttributeSets,
      $current->AttributeSets->children('ns2', true)) // Function Return Value

However as mseancole commented you can set the components as variables.

if (isset($current->AttributeSets))
{
    $children = $current->AttributeSets->children('ns2', true);

    if (isset($children,
              $children->ItemAttributes,
              $children->ItemAttributes->ListPrice, // etc.


E-Commerce 'sale price' calculator I have code which I need to refactor in order to use in many places. So I tried some solutions but always ending with repeated and messy code. So, I decided to ask what possible best solutions are.

This code is used for calculating sale price in e-commerce project. My goal is to put some code in method which will not change over time, or better to say, which will be managed only from one place. This part is considering of setting sale price based on some comparison. And problem occurs in this comparison. I want also to do some formatting of controls like Label based on this result, like setting currency code. The currency code will be sometimes in $, sometimes in USD. So, this means I should somehow isolate this currency code also.

In short, I want to refactor this currency code and formatting controls based on calculated sale price.

So, I created BasketHelper class with Product and Account properties and method SetBasketPayment that return properties I set in this method. Basically, I use this approach to group properties of Product and Account classes and then return values.

Here is my code. Any further explanation, I will provide upon your request.

public class BasketHelper
{
    public Product _product { get; set; }
    public Account _account { get; set; }

    public BasketHelper SetBasketPayment(Product product, Account account,
        HyperLink lblIconOnSale, Label lblSalePrice, Label lblListPrice,
        HyperLink lblIconCampaign)
    {
        decimal _brandAccountDiscountRate = default(decimal);
        decimal _accountDiscount = default(decimal);

        if (HttpContext.Current.Request.IsAuthenticated) {
            MembershipUser mUser = Membership.GetUser();
            if (mUser != null) {
                account = Account.GetAccountByUserId(
                    (Guid)Membership.GetUser().ProviderUserKey);
                try {
                    _accountDiscount = account.DiscountRate;
                } catch (Exception ex) { }

                BrandAccountDiscount brandAccountDiscount =
                    BrandAccountDiscount
                        .GetBrandAccountDiscountByUserAndBrandId(
                            product.BrandId, mUser.ProviderUserKey.ToString());
                if (brandAccountDiscount != null) {
                    _brandAccountDiscountRate = brandAccountDiscount.DiscountRate;
                }
            }
        }

        decimal currencyMultiplier = Currency.GetCurrencyValue(product.CurrencyCode);
        decimal _listPriceTL = product.ListPrice * currencyMultiplier;

        decimal _productCampaignPrice = _listPriceTL * (1 - product.DiscountRate / 100);
        decimal _accountPrice = _listPriceTL * (1 - _accountDiscount / 100);
        decimal _brandPrice = _accountPrice * (1 - _brandAccountDiscountRate / 100);

        lblListPrice.Text = product.ListPrice.ToString("N2") + " " + product.CurrencyCode;

        if (product.DiscountRate > 0) {
            product.SalePrice = _productCampaignPrice;

            lblSalePrice.Text = _productCampaignPrice.ToString("C2") + " + KDV";
            lblListPrice.CssClass += " strike";
            lblIconCampaign.Text = "+%" + product.DiscountRate.ToString("N0");
            lblIconCampaign.Visible = true;
        } else {
            if (_accountPrice < _listPriceTL) {
                product.SalePrice = _accountPrice;

                lblIconOnSale.Text = "%" + _accountDiscount.ToString();
                lblIconOnSale.Visible = true;
            }

            if (_brandAccountDiscountRate > 0) {
                product.SalePrice = _brandPrice;

                lblSalePrice.Text = _brandPrice.ToString("C2") + " +KDV";
            }
        }

        return new BasketHelper {
            _product = product,
            _account = account
        };
    }
}

BasketHelper mixes the different aspects of identifying and authenticating users, calculating prices and displaying information. These aspects should be separated. Especially manipulating the view (setting label texts, choosing CSS-formatting etc.) should not be mixed with business logic. The view logic shouldn't need to calculate prices and discounts. It should only work on results calculated somewhere else.

You have a Product class. Why do you not move product related calculations to Product? For instance add a method GetProductCampaignPrice to Product. You could make it a property as well, but since it is not returning a "static" value but depends on other properties, a method seems more appropriate.

public decimal GetProductCampaignPrice()
{
    decimal currencyMultiplier = Currency.GetCurrencyValue(CurrencyCode);
    decimal listPriceTL = ListPrice * currencyMultiplier;
    return listPriceTL * (1 - DiscountRate / 100);
}

Except for the currency calculator the calculation only needs properties of the product itself (CurrencyCode, ListPrice and DiscountRate). An even better design would be to inject a currency calculator into this method in order to remove the dependency on a specific currency calculator.

public decimal GetProductCampaignPrice(ICurrencyCalculator currencyCalculator)
{
    decimal listPriceNormalized = currencyCalculator.ToStandardCurrency(ListPrice, CurrencyCode);
    return listPriceNormalized * (1 - DiscountRate / 100);
}

Also note that I have removed the leading underscore (_) from the local variable. Underscores are used for fields, i.e. "variables" at class or struct level only.

The stuff in helper classes should be kept to a strict minimum and should really on hold things that don’t fit in anywhere else.

Look at your code and try to identify things that you can move to more specific classes.

A possible class integrating product and account related calculations could be called PriceCalculator; this is a much better name than FooHelper and it leads to a better design. Naming is crucial and goes hand in hand with good designs.

Note: If you don't want to or cannot change the Product class (e.g. because it is generated automatically), you could split the class into two partial classes, one holding the data and one with additional methods. You could also create a ProductExtensions class and make the method an extension method

public static class ProductExtensions
{
    public static decimal GetProductCampaignPrice(this Product product,
                                                  ICurrencyCalculator currencyCalculator)
    {
        decimal listPriceNormalized =
            currencyCalculator.ToStandardCurrency(ListPrice, CurrencyCode);
        return listPriceNormalized * (1 - DiscountRate / 100);
    }
}

You can call this method as if it was a member of Product

decimal result = product.GetProductCampaignPrice(currencyCalculator);


Alternative for storing id in hidden input Normally, when editing something, say when admin is approving a comment to a product, the id of the comment is stored in <input type="hidden"> to be included during form submission. But with this, just a change to the id may affect other pending comment, or those that are already approved.

So instead I used password_hash to hash the comment id, and stores it with the original comment id in a session array. Then after submitting the form, I check the submitted hashed comment id to the session array, looking for a match, then getting the original comment id.

Here's how I generate the hashed comment id:

foreach ($pendingComments as $comment) {
    $commentId = $comment['Id'];
    $hashed = password_hash($commentId, PASSWORD_DEFAULT);
    $_SESSION['COMMENT_IDS'][] = ['hash'=>$hashed, 'id'=>$commentId];
    $customerName = $comment['GivenName'] . ' ' . $comment['Surname'];
    $message = $comment['Message'];
    $timestamp = $comment['Timestamp'];
    echo '

            ' . $customerName . '
            ' . $message . '
            ' . $timestamp . '

                <form action="" method="POST">
                    <input type="hidden" value="'.$hashed.'" name="commentId">
                    <button type="submit" class="btn btn-outline-success btn-sm" name="changeCommentStatus" value="1">Approve</button>
                    <button type="submit" class="btn btn-outline-danger btn-sm" name="changeCommentStatus" value="2">Reject</button>
                </form>

    ';
}

Here's how I check for a match after submission:

if (isset($_POST['changeCommentStatus'])) {
    $hashedId = $_POST['commentId'];
    $status = $_POST['changeCommentStatus'];
    $commentId = '';
    foreach ($_SESSION['COMMENT_IDS'] as $data) {
        $hash = $data['hash'];
        $id = $data['id'];
        if ($hashedId === $hash) {
            $commentId = $id;
            break;
        }
    }
    if (empty($commentId)) {
        // this means that the $hashedId didn't matched anything, which also means that it has been altered
        // redirect back to the page and says that the id is invalid
    }
    echo $commentId;die;
}
$_SESSION['COMMENT_IDS'] = [];

As of now, it is working. But I'm not sure if there are any critical disadvantage /drawbacks or something, or if this is not recommended at all.

Note:

Some may say that an admin will not intentionally alter the source or something, but I'm planning to use this every time I need to use <input type="hidden" >
This may be more useful if while a user edits his comment, and somehow changed the comment id to another comment id that also belongs to the same user, instead the other comment has been edited
I can somehow prevent the action if the replaced comment id belonged to another user, preventing the current user from indirectly editing other users' comments
And of course, this is also to be used for other content that uses <input type="hidden">, especially when editing, not just for comments

I would question the premise.

  But with this, just a change to the id may affect other pending comment, or those that are already approved.

I don't see a problem here.

Given the admin is allowed to approve any comment, and nobody else is authorized to access this page completely, I don't see how it's even a problem.

To put it bluntly, nobody in the world is using such a clumsy approach as it's either useless or has a completely different solution (to ensure that a user cannot access other users' records there should be an ACL system in the database, not some HTML/Session ratatouille).


InterviewStreet Triplet optimization I came up with a solution for the InterviewStreet problem "Triplet"

In short, there is an integer array d which does not contain more than two elements of the same value. How many distinct ascending triples (d[i] < d[j] < d[k], i < j < k) are present?

It successfully passes 9 test cases of 15. but my solution exceeds time limit in other test cases.

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Scanner;
import java.util.Set;

/**
 *
 * @author Dumindu
 */
public class Triplets {
    public static void main(String args[])
    {
        int[] arr,helper;
        Scanner scn=new Scanner(System.in);
        int x=scn.nextInt();
        arr=new int[x];
        helper=new int[x];
        for(int i=0;i<x;i++)
        {
            arr[i]=scn.nextInt();
        }
        int triplets=0;
        helper[0]=0;
        Map<Integer,Integer> map1=new HashMap<Integer,Integer>();

        for(int i=0;i<x;i++)
        {
            int minVals=0;

            Set<Integer> set=new HashSet<Integer>();
            int tempTrip=0;
            for(int j=i-1;j>=0;j--)
            {
                if(arr[j]<arr[i] && !set.contains(arr[j]))
                {
                    minVals++;
                    tempTrip+=helper[j];
                    set.add(arr[j]);
                }
            }
            helper[i]=minVals;
            if(!map1.containsKey(arr[i]))
            {
                triplets+=tempTrip;
                map1.put(arr[i],tempTrip);
            }
            else
            {
                triplets=triplets-map1.get(arr[i])+tempTrip;
            }
        }
        System.out.println(triplets);
    }

}

If there is a way to optimize this solution or approach it differently, please let me know.

The problem looks like it would have a solution of complexity \$O(n log n)\$. Yours has \$O(n^2)\$. google "number of increasing subsequences".
The site says input size can be \$10^5\$. A \$O(n^2)\$ algorithm with hash look-ups, hash inserts, and virtual function calls in the inner loop will not finish in a few minutes.
By replacing

int x=scn.nextInt();

with

int x = 100000;

and

    arr[i]=scn.nextInt();

with

 arr[i] = i + 1;

you can easily test how your program performs with large input.
A quick order of magnitude analysis: An increasing sequence of ~\$10^5\$ elements will have \$~(10^5)^3\$ increasing subsequences of length 3. Since \$10^3 ~= 2^10\$, \$10^15 ~= 2^50\$. \$2^50\$ >> \$2^31\$. So your triplets variable will overflow. It should be an long.


Plain old quick sort Any suggestions for improvement are welcome.

public static int[] concatArrays(int[] a1, int[] a2) {
    int n1 = a1.length;
    int n2 = a2.length;
    if(n1 == 0) return a2;
    if(n2 == 0) return a1;

    int n = n1 + n2;
    int counter = 0;
    int[] result = new int[n];
    for(; counter < n1; counter++) {
        result[counter] = a1[counter];
    }
    for(; counter < n; counter++) {
        result[counter] = a2[counter - n1];
    }
    return result;
}

public static int[] concatArrays(int[] a1, int a2) {
    int[] a2New = new int[] {a2};
    return concatArrays(a1, a2New);
}

public static int[] sort(int[] data) {
    if(data.length == 0) return new int[0];
    int nBefore = 0;
    int nAfter = 0;
    int[] beforeArray = new int[data.length];
    int[] afterArray  = new int[data.length];
    for(int i = 1; i < data.length; i++) {
        if(data[i] < data[0]) {
            beforeArray[nBefore++] = data[i];
        }
        else {
            afterArray[nAfter++] = data[i];
        }
    }
    int[] beforeArrayTrimmed = new int[nBefore];
    int[] afterArrayTrimmed = new int[nAfter];
    System.arraycopy(beforeArray, 0, beforeArrayTrimmed, 0, nBefore);
    System.arraycopy(afterArray, 0, afterArrayTrimmed, 0, nAfter);
    return concatArrays(
            concatArrays(sort(beforeArrayTrimmed), data[0]),
            sort(afterArrayTrimmed)
    );
}

Using counter for both for loops seems awkward.

for(int i = 0; i < n1; i++) {
    result[i] = a1[i];
}
for(int i = 0; i < n2; i++) {
    result[n1 + i] = a2[i];
}

This way, the for loop looks just like people expect it to look. Your implementation requires extra thought. There is still a single arithmetic operation for indexing in the second loop, so reusing counter is not saving you anything.

One line if statements can be missed by someone reading your code. Plus you need to remember to add curly brackets if you want to add a second line.

concatArrays(int[] a1, int a2) does not concatenate arrays. Function names that lie about what they do are bad.


implement the merge functionality for 2048 with JavaScript I am trying to implement the game 2048 using JavaScript. I am using a two-dimensional array to represent the board. For each row, it is represented using an array of integers.
Here I am focused on implementing the merge left functionality i.e. the merge that happens after the user hits left on their keyboard.
Here are a set of test cases that I came up with
const array1 = [2, 2, 2, 0] //  [4,2,0,0]
const array2 = [2, 2, 2, 2] // [4,4,0,0]
const array3 = [2, 0, 0, 2] // [4,0,0,0]
const array4 = [2, 2, 4, 16] // [4,4,16,0]

Here is one workable solution:
function mergeLeft(array) {
    let startIndex = 0;
    for (let endIndex = 1; endIndex < array.length; endIndex++) {
        if (array[endIndex] === 0) continue;
        let target = array[startIndex];
        if (target === 0 || target === array[endIndex]) { // shift or merge
            array[startIndex] += array[endIndex];
            array[endIndex] = 0;
        } else if (startIndex + 1 < endIndex) {
            endIndex--; // undo the next for-loop increment
        }
        if (target !== 0) startIndex++;
    }
    return array;
}

This works but there are a lot of if-else checking that makes it hard to extend or adapt to merges in other directions e.g. right, up, down.
Is there a better way to design the algorithm or the function so it can be more easily extensible to other directions? Any other feedback is welcomed!
In addition, I would like to ask a few more questions:

If we are going to build this game using a traditional object-oriented way, what are the class we need to design here?
I figured that we can use a 2-dimensional array to represent the board, or as one of the answers presented, we can use a 1-dimensional array to hold x,y coordinates, I wonder what are some of the pros and cons with either approach?
I was looking at it from an OOD approach and I was also aware that we can use functional programming instead. Here is my attempt

const reverse = (array) => [...array].reverse()

const transpose = (grid) => grid[0].map((_, i) => grid.map((r) => r[i]))

const rotateClockwise = (grid) => transpose(reverse(grid))

const rotateCounter = (grid) => reverse(transpose(grid))

// helper functions
const shift = ([n0, n1, ...ns]) =>
  n0 == undefined
    ? []
    : n0 == 0
    ? shift([n1, ...ns])
    : n1 == 0
    ? shift([n0, ...ns])
    : n0 == n1
    ? [n0 + n1, ...shift(ns)]
    : [n0, ...shift([n1, ...ns])]

const fillZeros = row =>
    row.concat ([0, 0, 0, 0]) .slice (0, 4)

const merge = ([firstItem, secondItem, ...rest]) =>
    firstItem == undefined
      ? []
    : firstItem == 0
      ? merge ([secondItem, ...rest])
    : secondItem == 0
      ? merge ([firstItem, ...rest])
    : firstItem == secondItem
      ? [firstItem + secondItem, ... merge (rest)]
    : [firstItem, ...merge ([secondItem, ... rest])]

const mergeRow = (row) =>
    merge(row).concat([0, 0, 0, 0]).slice (0, 4)

const mergeLeft = (grid) => grid.map(mergeRow)
const mergeRight = (grid) => grid.map((row) => reverse(mergeRow(reverse(row))))
const mergeUp = (grid) => rotateClockwise(mergeLeft(rotateCounter(grid)))
const mergeDown = (grid) => rotateClockwise(mergeRight(rotateCounter(grid)))

However I cannot pinpoint exactly which approach i.e. functional programming vs. OOP is better. Can someone give me some concrete benefits for going with either approach?

Review
Some review points on your code

Always delimit code blocks with {}. Eg  if (target !== 0) startIndex++; is safer as         if (target !== 0) { startIndex++; }

Avoid using continue (Its a goto in disguise). Rather the line if (array[endIndex] === 0) continue; is better as if (array[endIndex]) { /*... loop code ...*/ }. See rewrite.

Try to reduce array indexing by storing values in a variable. See rewrite

0 is falsey so if (target !== 0) startIndex++; can be if (target) { startIndex++; } and if (target === 0 ||  can be if (!target ||

Variables that do not change should be constants. eg  let target = array[start]; can be const target = array[start];

Try to keep names short. Long names make code harder to read

Rewrite
rewriting your function using the points above.

function mergeLeft(row) {
    var start = 0;
    for (let end = 1; end < row.length; end++) {
        const tile = row[end];
        if (tile) {
            const target = row[start];
            if (!target || target === tile ) {
                row[start] += tile ;
                row[end] = 0;
            } else if (start + 1 < end) {
                end--;
            }
            if (target) { start++; }
        }
    }
    return row;
}

Coordinate transform
Use a single dimension (1D) array to store the game board. The position on the board can be calculated from a coordinate pair. x = 2, y = 1 is 3rd column 2nd row. The index in a 1D array is index = x + y * size where size is the number of columns (4 in this case)
Your merge function moves from left to right, then down (I assume) to the next row and does the same. You can create an inner (columns) and outer (rows) loop, and calculate the board index from the coordinate pairs x,y

for (y = 0; y < 4; y ++) {
    for (x = 0; x < 4; x ++) {
        index = x + y * 4;
        // merge row
    }
}

The above steps over the arrays in the same direction as your mergeLeft function
To have it handle different directions you can define some functions that convert (transform) the x, y position to the correct index for the 4 directions. Up, Right, Left, and Down
For Example
const size = 4;                             // width and height of board
const tileCount = size * size;
const board = new Array(tileCount).fill(0); // Create the array of tiles

// named directions. The name of the key press
const moves = {
    up: 0,
    right: 1,
    down: 2,
    left: 3,
};

// functions that convert x,y position into board index
const coordTransform = [
    (x, y) => y + x * size,                // up from top down
    (x, y) => (size - 1) - x + y * size,   // right for right to left
    (x, y) => y + ((size - 1) - x) * size, // down from bottom up
    (x, y) => x + y * size,                // left from left to right
];

Before merging the rows we get the direction function const move = coordTransform[moves.up];
Then in the loop we use that function to transform the coordinates so that we step over items in the correct direction for the move. const idx = move(x, y); gets the transformed position.
Example using the above code to merge in any direction.
// The comments and naming is in terms of rows from left to right. move(moves.left)
// Call the function with the direction index eg move(moves.up);
function move(dir) {
    // Tiles will stack to the left.
    // left is the idx of the next left free pos
    // prev holds the prev left pos and is used to merge values if same
    var x, y = 0, left, prev;

    // get the function to convert coord to board index
    const move = coordTransform[dir];  //  transform function for direction
    while (y < size) {
        left = x = 0;
        prev = -1;
        while (x < size) {
            const idx = move(x, y);  // Get the tile index using transform function
            const tile = board[idx]; // Get the tile value
            if (tile) {              // Does tile have a value
                board[idx] = 0;                  // Remove the tile from the board
                const idxLeft = move(left, y);   // Get next stack pos
                board[idxLeft] = tile;           // Set the tile value
                if (prev > -1) {                 // Is there a tile to left
                    if (tile === board[prev]) {  // Is tile to left the same
                        board[prev] += tile;     // Merge the tile values
                        board[idxLeft] = 0;      // Remove second tile,
                        left--                   // Move index back
                        prev = -1;               // Ensure only two merge at a time
                    } else { prev = idxLeft }    // If tile did not merge remember
                } else { prev = idxLeft  }       //    position of tile
                left ++;                         // Move stack pos to next tile
            }
            x++;     // step to next column
        }
        y++;         // step to next row.
    }
}

UPDATE Implementing the game.
The snippet is an implementation of the game without an in depth study of the original game.
It uses a partial OO approach

"If we are going to build this game using a traditional object-oriented way, what are the class we need to design here?"

Objects used

Note that JS uses Objects. You should not use the term class due to the significant differences between classes and objects in computer science.

I do not use the class syntax due to its very ill though out implementation (incomplete and a syntactical hack)

There is a factory function Game that encapsulates the game, with support code to add the UI for the particular environment (Code review snippet) placed outside Game.
The Game object uses closure to create private functions and properties. It also reduces the need to add the noisy this. and the even hackier this.#
Within Game is the Object Tile that uses the traditional OO JS prototype object style to implement the display and animation of tiles.
There are also various simple objects within Game
. states  Holds enumerated game states.
. moves Holds enumerated game actions (a move is a keyboard (swipe, or mouse, if implemented)) direction to move tiles
. moveTransform is the transform used to convert 2D tile positions to 1D tile array positions depending on the move direction. (See details in first part of answer)
Outside the Game object
The keyboard interface uses a simple static object keys via an immediately invoked factory function (using closure to encapsulate private function) that ensures the will only be one instance of the keyboard interface.
Question 2

"I figured that we can use a 2-dimensional array to represent the board, or as one of the answers presented, we can use a 1-dimensional array to hold x,y coordinates, I wonder what are some of the pros and cons with either approach?"

The main reason is to reduce complexity. Rather than have to deal with coordinate pairs only a single value is needed to locate a tile. This reduces the complexity .
This game only has a small number of tiles 16 (though example allows up to 64 (it will handle many more but has be limited in the example to prevent poor performance on low end devices).
Many games use 2D layouts with 1000s of tiles, using 1D rather than 2D has many advantages

1D arrays reduce the overhead associated with indexing arrays items

1D arrays use less memory

1D arrays can easily be converted to typed arrays without the need for complex instantiation code. On low end devices a typed array (as a shared array) allows logic in worker threads to share the data.

Question 3

"I was looking at it from an OOD approach and I was also aware that we can use functional programming instead. Here is my attempt"

Functional programming (FP) as a style in JS adds way too much processing and memory overheads to make it a practical for realtime games.
FP in JS has been adopted as a defense against poor encapsulation that is so common in JS.
Using a robust encapsulation style there is (in my view) no need to write in FP style in JS
The example below is written to fit the code review snippet and as such can not use modules.
Modules add an additional layer of encapsulation to JS code preventing side effects by creating privates scopes per module further reducing the need to use the FP style.

const eCurve = (v, p = 2) =>  v <= 0 ? 0 : v >= 1 ? 1 : v ** p / (v ** p + (1 - v) ** p);
const bellCurve = (v, p = 2) =>  v <= 0 ? 0 : v >= 1 ? 0 : (v *= 2, (v = v > 1 ? 2 - v : v),  v ** p / (v ** p + (1 - v) ** p));
const bCurve = (v, a, b) => v <= 0 ? 0 : v >= 1 ? 1 : 3*v*(1-v)*(a*(1-v)+b*v)+v*v*v; // cubic bezier curve
Math.PI90 = Math.PI / 2;
Math.TAU = Math.PI * 2;
Math.PI270 = Math.PI * (3 / 2);
const randUI = (min, max) => (max !== undefined ? Math.random() * (max - min) + min : Math.random() * min) | 0;
const randPick = arr => arr.splice(randUI(arr.length), 1)[0];
const randItem = arr => arr[randUI(arr.length)];
const tag = (tag, props = {}) => Object.assign(document.createElement(tag), props);

canvas.width = innerWidth;
canvas.height = innerHeight;
const ctx = canvas.getContext("2d");
const keys = (()=>{
    const keys = {
        ArrowLeft: false,
        ArrowRight: false,
        ArrowUp: false,
        ArrowDown: false
    };
    document.addEventListener("keydown", keyEvent);
    document.addEventListener("keyup", keyEvent);
    function keyEvent(e) {
       if (keys[e.code] !== undefined) { keys[e.code] = e.type === "keydown"; e.preventDefault() }
    }
    return keys;
})();
var pageResized = false, inGame = false, instructionsKnown = 0, gameType = 1;
addEventListener("resize", () => { if (inGame) { pageResized = true } });
sel.addEventListener("click", (e) => {
  if (!inGame && e.target.dataset.game !== undefined) {
      gameType = e.target.dataset.game;
      sel.style.display = "none";
      playGame(gameType);
  }

});

const pathRoundBox = (ctx, x, y, w, h, r) => {
    const w2 = w / 2, h2 = h / 2;
    ctx.arc(x - w2 + r, y - h2 + r, r, Math.PI, Math.PI270);
    ctx.arc(x + w2 - r, y - h2 + r, r, Math.PI270, Math.TAU);
    ctx.arc(x + w2 - r, y + h2 - r, r, 0, Math.PI90);
    ctx.arc(x - w2 + r, y + h2 - r, r, Math.PI90, Math.PI);
}
function Game(size, winTile, speed = 20, tilesPerMove = 2) {
    var state;
    var moveCount = 0, displayDirty = true;
    const animRate = 1 / speed;
    const tileCount = size * size;
    const board = new Array(tileCount).fill(0);
    const mergingTiles = [];
    const freeTiles = [];
    const newTileVal = [];
    const newTileSet = [2,4,2,4,2,4,2,4,2,4,2,4];
    const states = {
        over: 1,
        inPlay: 2,
        animating: 3,
        win: 4,
    };
    state = states.inPlay;
    const moves = {
        up: 0,
        right: 1,
        down: 2,
        left: 3,
    };
    const moveTransform = [
        (x, y) => y + x * size,
        (x, y) => (size - 1) - x + y * size,
        (x, y) => y + ((size - 1) - x) * size,
        (x, y) => x + y * size,
    ];
    function Tile(val, idx, growDelay = 0) {
        this.nx = this.x = idx % size;
        this.ny = this.y = idx / size | 0;
        this.val = val;
        this.anim = 0;
        this.alpha = 1;
        this.animate = true;
        this.merging = false;
        this.grow = true;
        this.size = -growDelay;
        this.flash = false;
        this.flashAlpha = 0;
    }
    Tile.prototype = {
        newPos(idx) {
            this.nx = idx % size;
            this.ny = idx / size | 0;
            this.ax = this.x;
            this.ay = this.y;
            this.anim = 1;
            this.animate = true;
        },
        merge(idx) {
            this.newPos(idx);
            this.merging = true;
            return this;
        },
        mergeFlash(idx) {
            this.animate = true;
            this.flash = true;
            this.flashAlpha = 1;
            this.val *= 2;
            return this;
        },
        update() {
            if (this.merging) {
                this.alpha -= animRate;
                if (this.alpha <= 0) {
                    this.alpha = 0;
                    this.merging = false;
                }
            }
            if (this.anim > 0) {
                this.anim -= animRate;
                if (this.anim <= 0) { this.anim = 0 }
            }
            if (this.grow) {
                this.size += animRate;
                if (this.size > 1) {
                    this.size = 1;
                    this.grow = false;
                }
            }
            if (this.flash) {
                this.flashAlpha -= animRate * 2;
                if (this.flashAlpha < 0) {
                    this.flashAlpha = 0;
                    this.flash = false;
                }
            }
            this.animate =  this.flash || this.grow || this.merging || this.anim > 0;
            displayDirty = this.animate ? true : displayDirty;
            if (this.anim > 0) {
                const p = eCurve(1 - this.anim);
                this.x = (this.nx - this.ax) * p + this.ax;
                this.y = (this.ny - this.ay) * p + this.ay;
            } else {
                this.x = this.nx;
                this.y = this.ny;
            }
        },
        render(target) {
            const {ctx, scale, radius, cols, textCol, pad} = target;
            ctx.save()
            const hS = scale / 2;
            var x = this.x * scale;
            var y = this.y * scale;
            const inScale = this.flash ?  1 + bellCurve(this.flashAlpha) * 0.1 : bCurve(this.size, 0, 1.6);
            ctx.transform(inScale, 0, 0 , inScale, x + hS, y + hS);
            ctx.globalAlpha = eCurve(this.alpha);
            ctx.fillStyle = cols[this.val];
            ctx.beginPath();
            pathRoundBox(ctx, 0, 0, scale - pad * 2, scale - pad * 2, radius);
            ctx.fill();
            if (this.flash) {
                ctx.globalCompositeOperation = "lighter";
                ctx.globalAlpha = eCurve(this.flashAlpha);
                ctx.fillStyle = "#FFF";
                ctx.fill();
                ctx.globalCompositeOperation = "source-over";
                ctx.globalAlpha = 1;
            }
            ctx.fillStyle = textCol;
            ctx.fillText(this.val, 0, 0 + scale * 0.05);
            ctx.restore()
        }
    }
    function addTile(growDelay) {
        const pos = randPick(freeTiles);
        if (newTileVal.length === 0) { newTileVal.push(...newTileSet) }
        board[pos] = new Tile(randPick(newTileVal), pos, growDelay);
    }
    function checkBoard() {
        var i = tileCount, s = state;
        freeTiles.length = 0;
        while (i--) {
            const t = board[i];
            if (t?.val === winTile) {
                s = states.win;
            } else if (t === 0) {
                freeTiles.push(i);
            }
        }
        if (freeTiles.length < tilesPerMove && s !== states.win) { s = states.over }
        API.state = s;
    }
    function move(dir) {
        moveCount ++;
        var x, y = 0, f, prev;
        const move = moveTransform[dir];
        while (y < size) {
            f = x = 0;
            prev = -1;
            while (x < size) {
                const idx = move(x, y);
                const tile = board[idx];
                board[idx] = 0;
                if (tile) {
                    const idxf = move(f, y);
                    board[idxf] = tile;
                    idxf !== idx && tile.newPos(idxf);
                    if (prev > -1) {
                        if (tile.val === board[prev].val) {
                            board[prev].mergeFlash();
                            mergingTiles.push(tile.merge(prev));
                            board[idxf] = 0;
                            f--;
                            prev = -1;
                        } else { prev = idxf }
                    } else { prev = idxf }
                    f++;
                }
                x++;
            }
            y++;
        }
    }
    const API = {
        states: Object.freeze(states),
        moves: Object.freeze(moves),
        get displayDirty() {
            if (state === states.animating) { displayDirty = true }
            const res = displayDirty;
            displayDirty = false;
            return res;
        },
        get moveCount() { return moveCount },
        get state() { return state },
        set state(s) {
            if (s !== state) {
                if (state === states.animating && s === states.inPlay) {
                    state = s;
                    checkBoard();
                    if (state === states.inPlay) {
                        let i = 0
                        while (i < tilesPerMove) { addTile(i++ / 2) }
                        displayDirty = true;
                    }
                } else {
                    state = s;
                    displayDirty = true;
                }
            }
        },
        set move(dir) {
            if (state === states.inPlay) {
                if (dir >= 0 && dir < 4) {
                    move(dir);
                    API.state = states.animating;
                }
            }
        },
        reset() {
            board.fill(0);
            API.state = states.animating;
            moveCount = 0;
            checkBoard();
        },
        render(target) {
            var i = 0, tail = 0, animating = false;
            board.forEach(tile => {
                if (tile !== 0) {
                    tile.animate && (animating = true, tile.update());
                    tile.render(target);
                }
            });
            while (i < mergingTiles.length) {
                const tile = mergingTiles[i];
                tile.update();
                tile.render(target);
                if (tile.animate) {
                    animating = true
                    mergingTiles[tail++] = tile
                }
                i++;
            }
            mergingTiles.length = tail;
            !animating && state === states.animating && (API.state = states.inPlay);
        },
    }
    return Object.freeze(API);
}

function createBGImage(img, tiles, pad, radius) {
    const ctx = img.getContext("2d");
    ctx.fillStyle = "#222";
    ctx.beginPath();
    pathRoundBox(ctx, img.width / 2, img.height / 2, img.width, img.height, radius + pad * 2);
    ctx.fill();
    var i = 0;
    const size = img.width / tiles;
    ctx.fillStyle = "#666";
    while (i < tiles * tiles) {
        const x = (i % tiles) + 0.5;
        const y = (i / tiles | 0)  + 0.5;
        ctx.beginPath();
        pathRoundBox(ctx, x * size, y * size, size - pad * 2, size - pad * 2, radius);
        ctx.fill();
        i++;
    }
    return img;
}

function playGame(game = gameType) {
    inGame = true;
    const GAMES = [
       [4, 512,  20, 2],
       [5, 2048, 20, 2],
       [6, 2048, 20, 2],
       [7, 1024, 20, 3],
       [8, 2048, 16, 4],
    ];
    const SIZE = GAMES[game][0];
    const g = Game(...GAMES[game]);
    g.reset();
    ctx.canvas.width = innerWidth;
    ctx.canvas.height = innerHeight;
    requestAnimationFrame(animLoop);
    const minRes =  Math.min(ctx.canvas.width, ctx.canvas.height);
    const unit =  minRes / 64;
    const RADIUS = minRes / SIZE / 8, PAD = minRes / SIZE / 16;
    const bgImage = createBGImage(tag("canvas", {width: minRes, height: minRes}), SIZE, PAD / 2, RADIUS + PAD / 2);
    const renderTarget = {
        ctx,
        scale:  minRes / SIZE,
        radius: RADIUS,
        pad:    PAD,
        textCol: "#000",
        cols: {
            [2]:    "#F40",
            [4]:    "#F80",
            [8]:    "#FA0",
            [16]:   "#FC0",
            [32]:   "#FF0",
            [64]:   "#CF0",
            [128]:  "#8F0",
            [256]:  "#0F0",
            [512]:  "#0F8",
            [1024]: "#0FF",
            [2048]: "#FFF",
        }
    }
    ctx.font = (renderTarget.scale * 0.7 | 0) + "px Arial";
    ctx.textAlign = "center";
    ctx.textBaseline = "middle";
    function animLoop() {
        var m = -1;

        // Rather than dynamic resize this just restarts on resize
        if (pageResized) {
            pageResized = false;
            setTimeout(playGame, 0);
            inGame = false;
            return;
        }
        if (keys.ArrowUp) { keys.ArrowUp = false; m = g.moves.up }
        if (keys.ArrowRight) { keys.ArrowRight = false; m = g.moves.right }
        if (keys.ArrowDown) { keys.ArrowDown = false; m = g.moves.down }
        if (keys.ArrowLeft) { keys.ArrowLeft = false; m = g.moves.left }

        if (m > -1) {
            g.move = m;
            instructionsKnown++;
        }
        const state = g.state;
        if (g.displayDirty) {
            ctx.textAlign = "center";
            ctx.textBaseline = "middle";
            ctx.setTransform(1,0,0,1,0,0);
            ctx.clearRect(0, 0, ctx.canvas.width, ctx.canvas.height);
            ctx.font = (renderTarget.scale * 0.45 | 0) + "px Arial";
            ctx.setTransform(1,0,0,1,(canvas.width - SIZE * renderTarget.scale) / 2, 0);
            ctx.drawImage(bgImage, 0, 0);
            g.render(renderTarget);
            if (!(state === g.states.inPlay || state === g.states.animating)) {
                ctx.fillStyle = "#0008";
                ctx.setTransform(1,0,0,1,0,0);
                ctx.fillRect(0, 0, ctx.canvas.width,  ctx.canvas.height);
                ctx.font = (renderTarget.scale * 0.7 | 0) + "px Arial";
                ctx.textAlign = "center";
                ctx.textBaseline = "middle";
                ctx.fillStyle = "#FFF";
                ctx.strokeStyle = "#000";
                ctx.lineWidth = unit * 2;
                ctx.lineJoin = "round";
                const cx =  ctx.canvas.width * 0.5;
                const cy =  ctx.canvas.height * 0.3
                if (state === g.states.win) {
                    ctx.strokeText("WINNER!", cx + unit / 2, cy + unit / 2);
                    ctx.fillText("WINNER!",cx, cy)
                    ctx.strokeStyle = "#FED";
                    ctx.lineWidth = unit / 2;
                    ctx.lineJoin = "round";
                    ctx.strokeText("WINNER!", cx, cy)
                    ctx.font = (renderTarget.scale * 0.4 | 0) + "px Arial";
                    ctx.fillStyle = "#FFF";
                    ctx.strokeStyle = "#000";
                    ctx.lineWidth = unit;
                    const mC = g.moveCount;
                    ctx.strokeText("Moves: " + mC, cx + unit / 4, cy + unit * SIZE * 3 + unit / 4);
                    ctx.fillText("Moves: " + mC, cx, cy + unit * SIZE * 3);

                } else {
                    ctx.strokeText("GAME OVER", cx + unit / 2, cy + unit / 2);
                    ctx.fillText("GAME OVER", cx, cy)
                    ctx.strokeStyle = "#DEF";
                    ctx.lineWidth = unit  / 2;
                    ctx.lineJoin = "round";
                    ctx.strokeText("GAME OVER", cx, cy);
                }

                inGame = false;
                return;
            }
            if (instructionsKnown < 2) {
                ctx.setTransform(1,0,0,1,0,0);
                ctx.fillStyle = "#FFF";
                ctx.font = "16px Arial";
                ctx.fillText("Use arrow keys to play", ctx.canvas.width / 2, 20);
            }
        }
        requestAnimationFrame(animLoop);
    }
}
canvas {
   position: absolute;
   top: 0px;
   left: 0px;
   pointer-events: none;
}
#sel {
  font-family: arial;
  display: flex;
}
.bt {
   flex-wrap: row;
   cursor: pointer;
   width: 50px;
   text-align: center;
   border: 1px solid black;
   margin: 4px;
   padding-top: 3px;
}
.bt:hover {
   background: #D92;
}
.txt {
  margin-top: 8px;
}
<canvas id="canvas"></canvas>
<div id="sel">
   <div class="txt">Select grid size:</div>
   <div class="bt" data-game="0">4 by 4</div>
   <div class="bt" data-game="1">5 by 5</div>
   <div class="bt" data-game="2">6 by 6</div>
   <div class="bt" data-game="3">7 by 7</div>
   <div class="bt" data-game="4">8 by 8</div>
</div>

Note Code review pages use a lot of JS and as such impact the performance of snippets. To get a true feel of how the game plays copy the code to a stand alone page..
Note as there is a limit to code review answer size the above was coded to fit and as such lost a lot of code and reduced naming length to fit.


Searching within a data model using an array matching multiple terms I have a standard tableview app that displays information from a data model array that uses a custom class. There is a UISearchController to filter the data.

A simplified version of the custom class is shown. I use a computed property for a string that is checked when searching the data.

class myObject: NSObject {

    // Inherits from NSObject as I use NSCoding for persistence

    var id: String = UUID().uuidString // These are unique for each item

    var name: String = ""
    var type: String = ""
    var batch: String = ""

    var searchableString: String {
        // This is a concatenation of some of the properties
        return self.name + " " + self.type + " " + self.batch
    }

    // etc...
}

Every time the user types a new character into the search bar of the UISearchController, the following method is called, which filters the main data array into a filteredItems array. Initially, I used a very basic form where the entire search text had to match.

func searchItems() {
    if let searchText = searchController.searchBar.text {
        filteredItems = allItems.filter { item in
            return item.searchableString.lowercased().contains(searchText.lowercased())
        }
    }
}

filteredItems is then displayed in the tableview.

I wanted to improve this so that each word typed in the search bar is searched for separately. I tried this, but of course it filters using OR logic (any word can match).

func searchItems() {
    if let searchText = searchController.searchBar.text?.split(separator: " ") {
        // Need to convert substring from split back to string
        let searchArray = searchText.map() { substring in
            String(substring)
        }
        filteredItems = allItems.filter { item in
            return searchArray.contains(where: drug.searchableString.lowercased().contains)
        }
    }
}

In order to get the desired AND logic (every word typed must match), I had to use this.

func searchItems() {
    if let searchText = searchController.searchBar.text?.split(separator: " ") {
        // Need to convert substring from split back to string
        let searchArray = searchText.map() { substring in
            String(substring)
        }
        filteredItems = allItems.filter { item in
            for word in searchArray {
                if !item.searchableString.lowercased().contains(word) {
                    return false
                }
            }
            return true
        }
    }
}

This works as intended, but it does not seem elegant. Is there a better way to filter the array allItems to filteredItems based on matching text in those String properties of the objects in my data model?

You have picked a hard data structure and hard algorithm to solve it.  Specifically, you have your searchable list as a list of strings and need to parse and convert it for each search.  You then do linear searches.

If you are going to precompute or cache your search information, consider creating a dictionary where the keys are the words and the values are sets of item indices.  Then you search is anding the set of indices with the value of each word.

Alternately, if you are optimizing for code brevity, use a regular expression. If your search strings were like ':MyName:MyType:MyBatch:' You want to create an expression from your search like :(word1|word2|word3): and use

NSUInteger numberOfMatches = [regex numberOfMatchesInString:string
                                                    options:0
                                                      range:NSMakeRange(0, [string length])];

and make sure it matches the number of words.   Yes, more work if you want to avoid the, er, inventive user, searching for "MyBatch MyBatch MyBatch".

Keep trying to make it prettier :)


Query with multiple foreign keys to the same table I have a primary table, REP_ASSIST.ES_LOG with multiple columns that are foreign keys to another table, REP_ASSIST.ES_DOCUMENT_STATUSES. Here is an example of what my main table looks like:

+-------------------------+-----------+-------------------------+-----------------------+
|       created_at        | filing_id | prior_dd_rcvd_status_id | new_dd_rcvd_status_id |
+-------------------------+-----------+-------------------------+-----------------------+
| 2019-09-04 10:58:48.000 |       988 | 2                       |                     2 |
| 2019-09-04 10:47:03.000 |       988 | 1                       |                     2 |
| 2019-08-28 23:56:47.000 |       988 | null                    |                     1 |
+-------------------------+-----------+-------------------------+-----------------------+

Both the prior_dd_rcvd_status_id and new_dd_rcvd_status_id are foreign keys to the other table, a sample of which is here:

+------------------+--------------------+
| dd_doc_status_id | dd_doc_status_name |
+------------------+--------------------+
|                1 | RECEIVED           |
|                2 | MISSING            |
|                3 | NOT_NEEDED         |
|                4 | UNKNOWN            |
+------------------+--------------------+

I need to pull in the values from the lookup table for a printable report; right now I'm using a subquery to get the dd_doc_status_name as a user-friendly name instead of the dd_doc_status_id.

SELECT created_at,
       filing_id,
       (
           SELECT dd_doc_status_name
           FROM REP_ASSIST.ES_DOCUMENT_STATUSES
           WHERE ES_DOCUMENT_STATUSES.dd_doc_status_id = prior_dd_rcvd_status_id
       ) AS 'prior_dd_status',
       (
           SELECT dd_doc_status_name
           FROM REP_ASSIST.ES_DOCUMENT_STATUSES
           WHERE ES_DOCUMENT_STATUSES.dd_doc_status_id = new_dd_rcvd_status_id
       ) AS 'new_dd_status'
FROM REP_ASSIST.ES_LOG
WHERE filing_id = 988;

However, I'm concerned that using subqueries like this may be a bit of a hack and affect performance (especially if I end up adding additional columns that need this information).

I'm getting the correct results (see below), but I'd like to know if there is a more efficient and more standard way.

+-------------------------+-----------+----------------------+--------------------+
|       created_at        | filing_id | prior_dd_rcvd_status | new_dd_rcvd_status |
+-------------------------+-----------+----------------------+--------------------+
| 2019-09-04 10:58:48.000 |       988 | MISSING              | MISSING            |
| 2019-09-04 10:47:03.000 |       988 | RECEIVED             | MISSING            |
| 2019-08-28 23:56:47.000 |       988 | null                 | RECEIVED           |
+-------------------------+-----------+----------------------+--------------------+

I've updated the Fiddle with your initial query and 2 alternatives.

This is the initial query plan:

Using OUTER APPLY instead of the nested SELECT statements.

select created_at, prior_dd_rcvd_status_id,
    new_dd_rcvd_status_id, prior_dd_status, new_dd_status, filing_id
from ES_LOG
outer apply (
    select dd_doc_status_name as prior_dd_status
    from ES_DOCUMENT_STATUSES where dd_doc_status_id = prior_dd_rcvd_status_id
) a
outer apply (
    select dd_doc_status_name as new_dd_status
    from ES_DOCUMENT_STATUSES where dd_doc_status_id = new_dd_rcvd_status_id
) b
where filing_id = 988;

The query plan gets simplified:

And using LEFT OUTER JOIN instead of the OUTER APPLY.

select created_at, prior_dd_rcvd_status_id,
    new_dd_rcvd_status_id,
    a.dd_doc_status_name as prior_dd_status,
    b.dd_doc_status_name as new_dd_status,
    filing_id
from ES_LOG
left outer join ES_DOCUMENT_STATUSES a
    on a.dd_doc_status_id = prior_dd_rcvd_status_id
left outer join ES_DOCUMENT_STATUSES b
    on b.dd_doc_status_id = new_dd_rcvd_status_id
where filing_id = 988;

With an even more simplified query plan:

However, as you can see, the table scans take most of the time. So I'm not sure how much performance you could gain by picking either of the alternatives.

Further improvements require the use of indexes (Table Scan vs Index Scan). Deciding which index to provide depends not only on this query, but also the general design of how these tables will be used for other queries and CRUD operations. I would advise you learn about them.


Calculating the factorial of any number I have created what was a rather simple piece of code to calculate the factorial of a number, although I wanted to ensure that my program could calculate the factorial of any given number. To do this, the only data type I thought was relevant was BigInteger. The problem I had with this was that my code quickly became quite messy, nevertheless, it works! I have chosen to take the approach of using recursion to calculate the answer, as I felt it is the best way to keep the code short and efficient.

I have looked at others code on this site, and read the reviews given, and it seems most users have chosen to use int, which isn't a problem, although as I say, I don't want an IntegerOverflow to occur when the user gives their input.

Is there a way to "clean up" this code; or make it more efficient?

import java.math.BigInteger;
public class Factorial {
    public static void main (String [] args) {
        java.util.Scanner sc = new java.util.Scanner(System.in);
        System.out.print("Enter an integer to calculate the factorial of: ");
        BigInteger temp = sc.nextBigInteger();
        sc.close();
        System.out.println(temp + "! = " + calculateFactorial(temp));
    }

    private static BigInteger calculateFactorial (BigInteger n) {
        if (n.compareTo((new BigInteger("1"))) ==  -1) {
            return new BigInteger("1");
        } else {
            return (n.multiply(calculateFactorial(n.subtract(new BigInteger("1")))));
        }
    }
}

Fully qualified class names are rarely written in the body of the code; they are almost always imported instead.

If you want to close the Scanner, use a try-with-resources block.  It calls close() even in the case of an intervening exception, and it is easier to see where the Scanner is valid.

temp is nearly never a good variable name.  Even n would be better: it's a conventional variable name for some kind of integer.

It would be good practice to suppress the default constructor by defining a private Factorial() {} constructor.

calculateFactorial would be better named factorial, and it should probably be public.

Are you really going to compute the factorial of a number larger than Long.MAX_VALUE?  You'll run into problems with stack overflow from recursion.  (You should use iteration instead.)  Even if that worked, you would still be dealing with numbers so large that I'm not sure BigInteger could handle.


Emptying and populating an element in jQuery I have written the following function to empty and then populate multiple elements using jQuery:

function drawHumanCard()
{
    var random = generateRandom();

    var humanEl = $('#human');

    humanEl.find('#human-name').empty().text(cards[random]['name']);
    humanEl.find('#human-job').empty().text(cards[random]['job']);
    humanEl.find('#human-image').attr('src', 'img\/trumps\/' + cards[random]['image']);
}

var cards = [
    new TopTrumpCard('Name 1', 'designer', 'image1.jpg'),
    new TopTrumpCard('Name 2', 'designer', 'image2.jpg'),
    new TopTrumpCard('Name 3', 'designer', 'image3.jpg')
];

function TopTrumpCard(name, job, image)
{
    this.name       = name;
    this.job        = job;
    this.image      = image;
}

function generateRandom()
{
    var objCount = Object.keys(cards).length - 1;
    var number   = Math.floor(Math.random() * (objCount - 0 + 1)) + 0;

    return number;
}

What the drawHumanCard function does:

var random is a randomly generated number between a specific range. The range is 0 and number of cards array keys.

cards is a variable that contains an array of data. The random variable is used to target a specific key in the cards array.

From then on, I've defined 3 elements. 2 of which require the text to be changed and 1 that requires the image src attribute value to be changed.

What would be the best way to write this in a cleaner way?

I was thinking about defining the jQuery element objects in an array and then running an each loop.

As you can see, I am using jQuery for this, but am happy to go with vanilla JS too.

Your code isn't bad, but there are some aspects that tick me a bit.

Lets start with the generateRandom()

You currently have the following:

var objCount = Object.keys(cards).length - 1;
var number   = Math.floor(Math.random() * (objCount - 0 + 1)) + 0;

return number;

And I ask:

Why + 0 or - 0?
Why Object.keys(cards).length - 1; for an array?
Why take 1 to sum it again?
Why that bloat?

Here's how I would write it:

function generateRandom()
{
    return Math.floor(Math.random() * cards.length);
}

Since the length is 1-based and the indexes are 0-based and Math.random() returns numbers between 0 and 1 (excluding the 1), you don't need more than this.
And now, the drawHumanCard()

You have the following code:

var random = generateRandom();

var humanEl = $('#human');

humanEl.find('#human-name').empty().text(cards[random]['name']);
humanEl.find('#human-job').empty().text(cards[random]['job']);
humanEl.find('#human-image').attr('src', 'img\/trumps\/' + cards[random]['image']);

You can store cards[random] inside a variable, like this:

var card = cards[random];

And to improve performance, you don't need the .empty(). It's a waste of time and CPU power.

Also, why humanEl.find('#human-name') instead of $('#human-name')? And why not an array with these objects?

I would rewrite the whole thing as this:

var human;

//it must run after the elements exist in the DOM
$(document).ready(function(){
    human = {
        name: $('#human-name'),
        job: $('#human-job'),
        image: $('#human-image')
    };
});

function drawHumanCard()
{
    var random = generateRandom();

    var card = cards[random];

    human.name.text(card['name']);
    human.job.text(card['job']);
    human.image.attr('src', 'img/trumps/' + card['image']);
}

So, the final code would be like this:

var human;

$(document).ready(function(){
    human = {
        name: $('#human-name'),
        job: $('#human-job'),
        image: $('#human-image')
    };
});

var cards = [
    new TopTrumpCard('Name 1', 'designer', 'image1.jpg'),
    new TopTrumpCard('Name 2', 'designer', 'image2.jpg'),
    new TopTrumpCard('Name 3', 'designer', 'image3.jpg')
];

function TopTrumpCard(name, job, image)
{
    this.name = name;
    this.job = job;
    this.image = image;
}

function drawHumanCard()
{
    var random = generateRandom();

    var card = cards[random];

    human.name.text(card['name']);
    human.job.text(card['job']);
    human.image.attr('src', 'img/trumps/' + card['image']);
}

function generateRandom()
{
    return Math.floor(Math.random() * cards.length);
}

This can and should be written as vanilla Javascript. The use of jQuery isn't justifiable here but that is an exercise for the O.P. to try.


Mirror image of a binary tree I wrote a small routines to create a mirror for any binary tree. I am just sharing my necessary routines instead of full class. could you please let me know your comments on this. I just curious to find the complexity of this but I am new to algorithms.

void tree_sample::mirror_image()
{
    create_mirror_for_left_subtree(root->left);
    create_mirror_for_right_subtree(root->right);
    tree_node* temp = 0;
    if(root->left)
        temp = root->left;
    root->left = root->right;
    if(temp)
        root->right = temp;
    else
        root->right = NULL;
}

void tree_sample::create_mirror_for_left_subtree(tree_node *temp)
{
    while(temp!=NULL)
    {
        do_mirror_for_inidividual_node(temp);
        temp = temp->left;
    }
}

void tree_sample::create_mirror_for_right_subtree(tree_node *temp)
{
    while(temp!=NULL)
    {
        do_mirror_for_inidividual_node(temp);
        temp = temp->right;
    }
}

void tree_sample::do_mirror_for_inidividual_node(tree_node *temp)
{
    tree_node* temp2 = 0;
    if(temp->left)
        temp2 = temp->left;
    temp->left = temp->right;
    if(temp2)
        temp->right = temp2;
    else
        temp->right= 0;
}

I think you have over complicated things and I don't think it works for the generic tree (as your create_mirror_for_[left|right]_subtree() only recurse down one side of the tree.

Why the test before a swap?

tree_node* temp = 0;
if(root->left)
    temp = root->left;    // If left is NULL you can still assign it
                          // to temp and get the same result.
root->left = root->right;
if(temp)
    root->right = temp;
else
    root->right = NULL;   // If temp is null you could just assign
                          // it to right then both sides of the branch
                          // are identical and they collapse into a
                          // single statement.

// Much easier to write as:
tree_node* temp = root->left;
root->left      = root->right;
root->right     = temp;

// Which of course can be replaced by std::swap
std::swap(root->left, root->right);

I assume you want to swap all nodes in the tree (otherwise it's not a mirror).

class Tree
{
    public:
        void mirrorTree()
        {
            root.mirrorTree(root);
        }
    private:
        void mirrorTree(Node* node)
        {
            if (node == nullptr) {
                return;
            }
            using std::swap;
            swap(node->left, node->right);
            mirrorTree(node->left);
            mirrorTree(node->right);
        }
};


Making DataTable results more readable I have the method below, which generates backtesting results. However, it's highly unreadable. What could you possibly suggest to me in order to make it more readable?
public DataTable GenerateResultTable(List<BacktestResult> data)
{
    var table = new DataTable();

    table.Columns.Add("Pair");
    table.Columns.Add("Buys");
    table.Columns.Add("Avg Profit %");
    table.Columns.Add("Cum Profit %");
    table.Columns.Add($"Tot Profit {_backtestOptions.StakeCurrency}");
    table.Columns.Add("Tot Profit %");
    table.Columns.Add("Avg Duration");
    table.Columns.Add("Wins");
    table.Columns.Add("Draws");
    table.Columns.Add("Losses");

    // the number of the pairs in the backtest results
    var maxOpenOrders = data.GroupBy(e => e.Pair).Count();

    // Split results for each pair
    var query = data.GroupBy(e => e.Pair).Select(e => new { Pair = e.Key, Count = e.Count(), Value = e });

    // in case there are no records to show
    // if there are no rows, it throws an exception..
    if (!query.Any())
    {
        table.Rows.Add("", "", "", "", "", "", "", "", "", "");
    }

    foreach (var result in query)
    {
        var _profitSum = result.Value.Sum(e => e.ProfitPercentage);
        var _profitTotal = _profitSum / maxOpenOrders;

        var key = result.Pair;
        var trades = result.Count;
        var profitMean = trades > 0 ? result.Value.Average(e => e.ProfitPercentage) : 0;
        var profitMeanPercentage = trades > 0 ? result.Value.Average(e => e.ProfitPercentage) * 100 : 0;
        var profitSum = _profitSum;
        var profitSumPercentage = _profitSum * 100;
        var profitTotalAbs = result.Value.Sum(e => e.ProfitAbs);
        var profitTotal = _profitTotal;
        var profitTotalPercentage = _profitTotal * 100;
        var avgDuration = trades > 0 ? Math.Round(result.Value.Average(e => e.TradeDuration)) : 0;
        var wins = result.Value.Count(e => e.ProfitAbs > 0);
        var draws = result.Value.Count(e => e.ProfitAbs == 0);
        var losses = result.Value.Count(e => e.ProfitAbs < 0);

        table.Rows.Add(key, trades, $"{profitMeanPercentage:f2}", $"{profitSumPercentage:f2}", $"{profitTotalAbs:f8}", $"{profitTotalPercentage:f2}",
            $"{TimeSpan.FromMinutes((double)avgDuration):h\\:mm\\:ss}", wins, draws, losses);
    }

    return table;
}

public class BacktestOptions
{
    public string StakeCurrency { get; set; }
    public List<string> Pairs { get; set; }
    public string StrategyName { get; set; }
    public decimal StakeAmount { get; set; }
    public decimal OpenFee { get; set; }
    public decimal CloseFee { get; set; }
}

public class BacktestResult
{
    public string Pair { get; set; }
    public decimal ProfitPercentage { get; set; }
    public decimal ProfitAbs { get; set; }
    public decimal OpenRate { get; set; }
    public decimal CloseRate { get; set; }
    public DateTime OpenDate { get; set; }
    public DateTime CloseDate { get; set; }
    public decimal OpenFee { get; set; }
    public decimal CloseFee { get; set; }
    public decimal Amount { get; set; }
    public decimal TradeDuration { get; set; }
    public SellType SellReason { get; set; }
}

appsettings.json
{
  "BacktestConfiguration": {
    "StakeCurrency": "USDT",
    "Pairs": [ "TRXUSDT" ],
    "StrategyName": "RsiStrategy",
    "StakeAmount": 1000,
    "OpenFee": 0.001,
    "CloseFee": 0.001
  }
}

Here are my observations:

List<BacktestResult> data: Please try to use more expressive names than data. It is way too general and it does not help the reader / maintainer of the code. (same apply to table and query)

Populating columns: Your implementation has a lot of repetition in it: tables.Columns.Add.

One way to reduce that by using the AddRange:

var table = new DataTable();
table.Columns.AddRange(new []
{
    new DataColumn("Pair"),
    new DataColumn("Buys"),
    new DataColumn("Avg Profit %"),
    new DataColumn("Cum Profit %"),
    new DataColumn($"Tot Profit {_backtestOptions.StakeCurrency}"),
    new DataColumn("Tot Profit %"),
    new DataColumn("Avg Duration"),
    new DataColumn("Wins"),
    new DataColumn("Draws"),
    new DataColumn("Losses"),
});

We are almost there, here we have repeated the new DataColumn. With the following technique you can separate data and logic:
var columns = new[]
{
    "Pair", "Buys", "Avg Profit %", "Cum Profit %", $"Tot Profit {_backtestOptions.StakeCurrency}",
    "Tot Profit %", "Avg Duration", "Wins", "Draws", "Losses"
};

table.Columns.AddRange(columns.Select(name => new DataColumn(name)).ToArray());

data.GroupBy(e => e.Pair): This has been called twice, but unnecessarily.

Materialize it by calling the ToList or ToArray on it:
With this approach we could get rid of the maxOpenOrders variable.

var resultsGroupByPair = data.GroupBy(e => e.Pair).ToArray();
var query = resultsGroupByPair.Select(br => new { Pair = br.Key, Count = br.Count(), Value = br });
//...
var _profitTotal = _profitSum / resultsGroupByPair.Length;

query: Same applies here, Possible multiple enumeration

Materialize it by calling the ToList or ToArray on it

if there are no rows, it throws an exception..: This comment is not align with the code. Try to comment the Whys and Why nots, not the what and how. The latter two should be clear by the code itself, but the chosen strategy (and the excluded alternatives) might not be obvious from the code.
profitMean, profitSum, profitTotal: They are unused. If they are not needed then delete them.
trades > 0 ?: This condition has been repeated several times.

Introduce a local variable and refer to that one instead: bool isTradesPositive

result.Value.Average, result.Value.Sum: Same as in the previous case. If you use the same expression multiple times then save its result into a variable.
wins, draws, losses: Instead do 3 iterations you can do it with a simple one.

For example with Aggregate:

var (wins, losses, draws) = result.Value.Aggregate((0, 0, 0), (outcome, e) =>
{
    if (e.ProfitAbs > 0) outcome.Item1++;
    else if (e.ProfitAbs < 0) outcome.Item2++;
    else outcome.Item3++;
    return outcome;
});

or
var (wins, losses, draws) = result.Value.Aggregate((0, 0, 0),
    ((int winCount, int loseCount, int drawCount) outcome, BacktestResult e) =>
    {
        if (e.ProfitAbs > 0) outcome.winCount++;
        else if (e.ProfitAbs < 0) outcome.loseCount++;
        else outcome.drawCount++;
        return outcome;
    });


Comment List using ReactJS Looking for some ideas as to how to improve the below react component.

var JSON_URL = "https://api.example.io/comments.json";
class CommentList extends React.Component {
    constructor() {
        super();
        this.state = { comments: [] }
    }
    componentDidMount() {
        $.ajax({
            url: JSON_URL,
            dataType: 'json',
            success: function(data) {
                this.setState({comments: data.comments});
            }.bind(this)
        });
    };
    render() {
        return <ul> {
            this.state.comments.map((comment) => {
                return <li>{comment.body}—{comment.author}</li>;
            })}
       </ul>;
    }
}
React.render(<CommentList />, document.getElementById('root'));

ES6 arrow function

Nice work utilizing functional programming (i.e. .map()) and the arrow function. One improvement here is that there isn't much of a need for a code block (i.e. { ....}) and return statement. So the callback to .map() in the render method can be reduced to a single line like below:

render() {
    return
 {
        this.state.comments.map((comment) =>
{comment.body}—{comment.author}
)
    }
;
}

jQuery - just for AJAX requests?

It appears that the only jQuery functionality used is its AJAX wrapper (i.e. $.ajax()). If that is all jQuery is used for, consider using a different technique like the fetch API (read this article for more info).

componentDidMount() {
    fetch(JSON_URL)
    .then(response => response.json())
    .then(data => this.setState({comments: data.comments}));

See this plunker for a demonstration.

Update

It has been mentioned that the fetch API has no support for aborting the request. If that is desired, there are other alternatives like Superagent (recommended by you might not need jQuery). See this updated plunker for an example of using superagent with its abort method in the componentWillUnmount method.


Combinational string search using Java I wrote a program that receives 2 strings (Input and Search) in the Search string. The sign '+' indicates that if the substring before the '+' exists in the Input string. (The search string cannot start with '+' and there cannot be a '+' followed by another '+').

Can you review it for best coding practices and efficiency?

boolean notgood = false;
boolean break1 = false;
boolean break2 = false;
int counter = 0;

if (search.charAt(0)=='+'||search.charAt(0)=='*') {

    System.out.println("Invalid search striNG.");
    continue;
}
//////////////////////////////////////////////
for (i=0; i<search.length() && notgood==false; i++) {

    if (search.charAt(i)!='*' && search.charAt(i)!='+') {
        counter++;
    }
    if (counter == search.length()) {
        System.out.println("Invalid SEARCH string.");
        notgood=true;
        break;
    }

    if ((search.charAt(i)=='*')||(search.charAt(i)=='+')) {

        if (i!=search.length()-1) {

            if ((search.charAt(i+1)=='*')||(search.charAt(i+1)=='+')) {

                System.out.println("INvalid search string.");
                notgood=true;
                break1=true;
            }
        }
    }
}
////////////////////////////////////////////
for (i=0; i<search.length() && !break1; i++) {

    int c=0;

    if (search.charAt(i)=='+') {

        String word = search.substring(0,i);

        for (int j=0; j<input.length() && !break2; j++) {

            if ((input.charAt(j) == word.charAt(c)) && c<word.length()) {
                c++;
            }
            if (c>=word.length()) {

                System.out.println("Search string matches input string.");
                break1=true;
                break2=true;

            }
        }
        if (c<word.length()) {
            System.out.println("Search string doesn't match input string.");
        }
    }

For example, for Input = 'abcd' and Search = 'ab+cd+', the result should be the strings match.

1.) Your search only looks for the first substring only (e.g. "ab"), it will never match the others given (e.g. "cd").
Why? Because You wrote:

String word = search.substring(0,i);

That would give us "ab+cd" for the second substring (maybe this is an error and you wanted to grab "cd"?)
Anyway, for the first substring found you are setting break1 and therefore exiting the loop.
But instead of setting break-variables, you can break directly by writing "break;". This  would be faster and makes your code more readable.

2.) Always make tests (write JUnit-Tests) for your code. You can post them, too. In many companies you cannot check in your code without a corresponding test with full code coverage.
If you have input="axb" and search="ab+", then it would match! Is this really intended? I think it's an error, and you would have found out if you wrote tests.

3.) Many times you wrote :

((search.charAt(i)=='*')||(search.charAt(i)=='+'))

This is a double DRY (Don't Repeat Yourself, clean code red grade) coding practice violation.
Just evaluate search.charAt() once and store it in a variable "currentChar". Then evaluate the whole expression once and store it in a boolean variable "isDelimiter".
Your code:

if (search.charAt(i)!='*' && search.charAt(i)!='+') {
    counter++;
}
...
if ((search.charAt(i)=='*')||(search.charAt(i)=='+')) {
...

would be reduced to:

char currentChar = search.charAt(i);
boolean isDelimiter = (currentChar=='*' || currentChar=='+');
if (!isDelimiter) {
    counter++;
}
...
if (isDelimiter) {
...

4.) Generally code is written once, but read on average about 10 times. That's why we have the coding practice to use meaningful names (clean code orange grade, source code conventions).
So change "break1" to "isInvalidSeachString", "break2" to "hasNoMatch", "notgood" to "bad", "counter" to "validCharacterCounter" and "c" to "matchingCharacterCounter"

5.) Follow the Single level of abstraction principle.(clean code, orange grade)
You are doing too much in a single method. You are verifying the correctness of the search string. Then you are tying to grab each substring and then match each substring.
Meaningful names are much better than a meaningless separator like "///////...".
Your structure should be following

public boolean isMatchingWholeSearch(String input, String search)
{
  boolean searchIsCorrect = hasNoStartingPlus(search) && hasNoDoublePlus(search);
  if (searchIsCorrect) searchForAllMatches(input, search);
}
private boolean searchForAllMatches(String input, String search)
{
  for (int i=0, max=search.length(); i<max; i++) { // performance optimized for-loop
      boolean isSubstringSuffix = (search.charAt(i)=='+');
      if (isSubstringSuffix) {
          String substring = getSubstringBefore(search, i);
          boolean isFound = searchForSingleMatch(input, substring);
          if (isFound) return true;
      }
  }
  return false;
}
private boolean searchForSingleMatch(String input, String search)
{
  // here goes your inner loop.
  ...
}
private boolean hasNoStartingPlus(String search)
{
   ...
}
private boolean hasNoDoublePlus(String search)
{
   ...
}

6.) For not reinventing the wheel, you could use java streams String.chars() with lambda expressions, or String.split() methods and regular expressions for verifying. That would make your code much more readable and very short. But maybe you can only use an old java version. Tell me if you can use a new one, and I'll give you code samples for each.