Checking a value before assigning it to variable Throughout my code, I find myself testing if the output of a function is undefined or blank before i use the output. In this case, getQuery checks if the property of an object is either blank or if the object key is not set at all. In both cases, the output can be ignored. But if the object key holds a value, then I want to use this output as the value for my variable.

var filter = ""
if ( typeof getQuery("filter") != 'undefined' && getQuery("filter") != "" ) filter = getQuery("filter")
if ( filter != "" ) createFilterView( filter )

There must be a more optimized way of doing this without the repetition?

I often set the output of the function to a variable first, in order to not have to perform the function several times. A bit faster, I assume, but also not very elegant.

var getQueryFilter_output = getQuery("filter")
if ( getQueryFilter_output != 'undefined' && getQueryFilter_output != "" ) var filter = getQuery_output

What would be a better way of doing this?

Indentation

I'm not quite sure if this is correct, but from what I know, I believe it is.

Indentation in JavaScript does not matter as, before your code is run, it is completely minimized - meaning it removes all comments and unnecessary spaces.

That being said, trying to crush your JavaScript into three lines does not make it any faster.

Calling

In your first example, you call the same function three times with the same arguments each time. This is extremely inefficient.

It would be optimal to call it once and save it in a variable.

Therefore, these lines:

var filter = ""
if ( typeof getQuery("filter") != 'undefined' && getQuery("filter") != "" ) filter = getQuery("filter")
if ( filter != "" ) createFilterView( filter )

Can become:

var filter = "";
var ret = getQuery("filter"); // for lack of a better variable name
if(typeof ret != 'undefined' && ret != "") filter = ret;
if(filter != "") createFilterView(filter)

(Yes, you fixed this in the second example)

Conditionals 1

In your first example, you check to see that the return value of getQuery isn't "undefined" and that it isn't "". If that passed, you take the return value and place it in a variable. Then, you go through another conditional to make sure that that variable isn't empty.

Well, since in your first conditional you already check to make sure that the return value isn't empty, why don't you put the body of the second conditional into the first one.

This:

if ( typeof getQuery("filter") != 'undefined' && getQuery("filter") != "" ) filter = getQuery("filter")
if ( filter != "" ) createFilterView( filter )

Becomes:

if(typeof getQuery("filter") != 'undefined' && getQuery("filter") != "") {
    filter = getQuery("filter")
    createFilterView(filter)
}

Conditionals 2

In the conditional statements in both of your examples, you check that the return value of getQuery is not "". You don't have to specifically specify that.

You can just remove the != "" part and it would work the same way.

The further reduce the size of your conditional, you can have your getQuery function actually return the value undefined (null would be better, however) instead of it returning a string of the word "undefined".

Then, you could reduce your conditional to:

if(getQuery("filter"))

Semicolons

In JavaScript, it is a good practice to close statements with a semicolon.

Example:

createFilterView(filter)

Should be:

createFilterView(filter);
                        ^
                        |
                    semicolon

Quotes

This isn't really important, but I just thought I'd point it out.

You are consistent in your code with your quotes. For some strings, you use single quotes and for others, you use double quotes.

It is best to stick to one.


Encryption/decryption program Here is a little encryptor I wrote, I'm just asking for people to tell me what is good practice and what's not. Also, I mean the code, not the actual encryption (because it's just a dumb thing).

#include "stdafx.h"
#include <iostream>
#include <stdexcept>

using namespace std;

int encrypter(int input);
int decrypter(int input);
bool whatToDoFunc();

int main()
{
    try
    {
        bool keepGoing = true;

        while (keepGoing)
        {
            bool success = whatToDoFunc(); char redo;
            cout << "Press 'y' to redo or anykey to exit" << endl;
            cin >> redo;
            if (success == true && (redo == 'y' || redo == 'Y'))
                keepGoing == true;
            else
                keepGoing = false;
        }
        }
    catch (runtime_error err)
    {
        cout << err.what() << endl;
        char exitCon;
        cout << "Enter anykey to exit" << endl;
        cin >> exitCon;
        return -1;
    }

    return 0;
}

bool whatToDoFunc()
{
    int input;
    char whatToDo;
    cout << "Welcome to the JeGo encrypter what do you want to do encrypt or     decrypy? Enter E or D\n" << endl;
    cin >> whatToDo;
    if (whatToDo == 'e' || whatToDo == 'E')
    {
        cout << "Enter the number you want to encrypt" << endl;
        cin >> input;
        int inputEncrypted = encrypter(input);
        cout << "Your number encrypted is " << inputEncrypted << endl;
    }

    else
    {
        if (whatToDo == 'd' || whatToDo == 'D')
        {
            cout << "Enter the already encrypted number" << endl;
            cin >> input;
            int numDecrypted = decrypter(input);
            cout << "Your number decrypted is " << numDecrypted << endl;
        }
        else
        {
            cerr << "Error" << endl;
            throw runtime_error("Your input was invalid");
        }
    }

return true;
}

int encrypter(int input)
{
    int inputEncrypted;
    inputEncrypted = ((((input * 2) + 7) + 5) + 8);
    if (inputEncrypted % 2 == 0)
        ++inputEncrypted;
    else
        inputEncrypted = inputEncrypted + (13 + 13) + (11 + 11);
    return inputEncrypted;
}

int decrypter(int inputEncrypted)
{
    int decryptedNum = 0;
    if (inputEncrypted % 2 == 1)
        --inputEncrypted;
    else
    {
        (11 - 11) - (13 - 13) - inputEncrypted;
    }
    decryptedNum = ((((inputEncrypted - 8) - 5) - 7) / 2);
    return decryptedNum;
}

Please tell me what I could do to make it better or just nicer design! :)

Bug?

It looks like there is a bug here:

else
{
    (11 - 11) - (13 - 13) - inputEncrypted;
}

After simplification, the expression becomes:

else
{
    -inputEncrypted;
}

... which does nothing, as the result is not assigned, the value of inputEncrypted is unchanged. If you wanted to negate the value of the variable, you would have to write like this:

else
{
    inputEncrypted = -inputEncrypted;
}

I don't know if this is your real intention (didn't look at your code close enough). But the original code does absolutely nothing, it's reduced to a statement whose result is not stored anywhere.

Use boolean expressions directly

This can be simplified:

if (success == true && (redo == 'y' || redo == 'Y'))
    keepGoing == true;
else
    keepGoing = false;

To this:

keepGoing = success && (redo == 'y' || redo == 'Y');

Notice how you don't need to write x == true, you can use boolean expressions directly.

Naming

A name like whatToDoFunc is obviously bad. Good function names are extremely important to understand how a program works.


Simple Command system Below I have coded a simple command system that passes a string to a method and checks if the first word split by (' ') is a key in the registered commands dictionary, if it is, it executes the class that is stored as the dictionary entry's value.

few examles of what is typed for commands

@mycommand hello
:ohnicecommand John
.setamount 10000

CommandHandler.cs:

using System;
using System.Collections.Generic;

namespace Sirius.Base.Game.Habbo.Commands
{
    using Commands;
    using Commands.Habbo;
    using Other.GameClients;

    internal class CommandHandler
    {
        private readonly Dictionary<string, ICommand> _commandsRegistered;

        internal CommandHandler()
        {
            _commandsRegistered = new Dictionary<string, ICommand>();

            RegisterCommands();
        }

        private void RegisterCommands()
        {
            RegisterCommand(":info", new InfoCommand());
        }

        public bool IsCommandInput(Player player, string input)
        {
            ICommand command;

            var cmdString = input.Split(' ')[0];

            if (!_commandsRegistered.TryGetValue(cmdString, out command))
            {
                return false;
            }

            if (command.PermissionRequired != "" && !player.GetPlayerData().GetPermissions().HasCommand(command.PermissionRequired))
            {
                return false;
            }

            if (command.NumberOfParameters < input.Split(' ').Length)
            {
                player.SendWhisper("Invalid command syntax -- " + cmdString + " " + command.CommandParameters);
                return true;
            }

            DateTime lastExecuted;

            if (player.GetPlayerData()._commandCooldowns.TryGetValue(cmdString, out lastExecuted))
            {
                var commandSpan = DateTime.Now - lastExecuted;

                if (command.CommandCooldown && commandSpan.TotalSeconds < (command.CommandCooldownMs / 1000))
                {
                    player.SendNotification("You're cooling down from this command! [" + commandSpan.TotalSeconds + "/" + (command.CommandCooldownMs / 1000) + "]");
                    return true;
                }
            }
            else
            {
                player.GetPlayerData()._commandCooldowns.Add(cmdString, DateTime.Now);
            }

            command.ProcessCommand(player);

            return true;
        }

        private void RegisterCommand(string commandName, ICommand command)
        {
            _commandsRegistered.Add(commandName, command);
        }
    }
}

ICommand:

namespace Sirius.Base.Game.Habbo.Commands.Commands
{
    using Other.GameClients;

    internal interface ICommand
    {
        void ProcessCommand(Player player);
        string PermissionRequired { get; }
        int NumberOfParameters { get; }
        string CommandParameters { get; }
        string CommandDescription { get; }
        bool CommandCooldown { get; }
        int CommandCooldownMs { get; }
    }
}

InfoCommand (just 1 of many commands)

using System;
using System.Text;

namespace Sirius.Base.Game.Habbo.Commands.Commands.Habbo
{
    using Other.Communication.Packets.Outgoing.Notifications;
    using Other.GameClients;

    internal sealed class InfoCommand : ICommand
    {
        public string PermissionRequired => "";
        public int NumberOfParameters => 0;
        public string CommandParameters => "";
        public string CommandDescription => "Lets you view information about the server.";
        public bool CommandCooldown => true;
        public int CommandCooldownMs => 5000;

        public void ProcessCommand(Player player)
        {
            // do what ever you want the command to do here...
        }
    }
}

The biggest issue I see is you have to manually register each and every command. Why bother? Why not just get a list of all commands that implement ICommand through Reflection? Speed shouldn't be a huge problem because I assume you do this on some sort of start-up.

var baseCommandInterface = typeof(ICommand);
var commands = AppDomain.CurrentDomain.GetAssemblies()
    .SelectMany(s => s.GetTypes())
    .Where(p => baseCommandInterface.IsAssignableFrom(p));

Now just enumerate commands and cast and operate on them. Life should be a lot easier this way.

Next, why bother with a bool and int for cool downs? Just use an int?, if it's null then there's no cool down, if its >0 then that's the cool down time. Pretty simple.

You use the term Command way too many times for my tastes.

internal interface ICommand
{
    void Process(Player player);
    string PermissionRequired { get; }
    int NumberOfParameters { get; }
    string Parameters { get; }
    string Description { get; }
    int? CooldownMs { get; }
}

Even then, NumberOfParameters is truly unnecessary. I'd bet there is a separator of some sort between commands in the Parameters string, which really shouldn't be a string, but should be something like the following:

public class CommandParameter
{
    public string Name { get; set; }
    public Type Type { get; set; }
    public string Description { get; set; }
}

Then Parameters becomes an IEnumerable<CommandParameter> which means you can use Parameters.Count in place of NumberOfParameters. This also means you can extrapolate further and define an abstract class for this, and add a CanProcess(Player player) method:

public abstract class BaseCommand : ICommand
{
    private List<CommandParameter> _parameters = new List<CommandParameter>();
    public IEnumerable<CommandParameter> Parameters => _parameters;

    public int? Cooldown { get; protected set; }
    public string Description { get; protected set; }
    public string PermissionRequired { get; protected set; }

    public abstract void ProcessCommand(Player player);

    public bool CanProcess(Player player)
    {
        // Check cool down, permissions, ban list, etc.
    }
}

Then in an actual command:

public class InfoCommand : BaseCommand
{
    public InfoCommand()
    {
        Cooldown = 5000;
        Description = "Lets you view information about the server.";
    }

    public override void ProcessCommand(Player player)
    {
        if (!CanProcess(player))
        {
            // I'm throwing an exception, but you could just `return` and send a message or whatever
            throw new InvalidOperationException("You have either exceeded the cool down, do not have permissions, or are banned. Please try again later.");
        }

        // Process our command here
    }
}

Then you created your own system that requires boilerplate code, and extracted that boilerplate code away.


OCaml rev_int function With my original code I kept getting Error: Parse error: [expr level ;] expected after "in" (in [expr]) on the line let numDigits = numDigits - 1 in

Original:

let rec rev_int num =
  if num / 10 == 0 then
     num
  else
    let temp = num mod 10 in

    let numDigits = String.length(string_of_int num) - 1 in

    if num < 0 then
      let numDigits = numDigits - 1 in
    else
      let numDigits = numDigits + 0 in

    let num = (num - temp) / 10 in
    temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num

With variations of:

if num < 0 then
   let numDigits = numDigits - 1 in;
else
   let numDigits = numDigits + 0 in;

if num < 0 then
   let numDigits = numDigits - 1 in
else begin
   let numDigits = numDigits + 0 in end

I revised the code and now it works, but I was wondering if there was a way to do it with nested if and less redundancy.

Revised:

let rec rev_int num =
  if num / 10 == 0 then
    num
  else
    let temp = num mod 10 in

    let numDigits = String.length(string_of_int num) - 1 in

    if num < 0 then
      let numDigits = numDigits - 1 in
      let num = (num - temp) / 10 in
      temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num
    else
      let numDigits = numDigits + 0 in
      let num = (num - temp) / 10 in
      temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num

It seems that in both branches you're doing the same calculation, so might want to begin by pushing the conditional in, obtaining:

let rec rev_int num =
  if num / 10 == 0 then
    num
  else
    let temp = num mod 10 in

    let numDigits = String.length(string_of_int num) - 1 in

    let numDigits = numDigits - (if num < 0 then 1 else 0) in
    let num = (num - temp) / 10 in
    temp * int_of_float(10.0 ** float_of_int numDigits) + rev_int num

Just an initial thought to get you started on improving the code.


Method for validating properties based on passed conditions I wrote a method for validating properties based on passed conditions. It works well, but declaration takes more space than the actual condition. Is there any way to make it shorter?

public virtual string Validate(string propertyName, params Tuple<Expression<Func<bool>>, string>[] predicates)
{
    if (!ValidationProperties.ContainsKey(propertyName))
        throw new ArgumentException($"No property found with name '{propertyName}'. Make sure you register it first.");

    foreach (var predicate in predicates)
    {
        if (predicate.Item1.Compile().Invoke())
        {
            ValidationProperties[propertyName] = false;
            ValidationChangedEvent?.Invoke(this, false);
            return predicate.Item2;
        }
    }

    ValidationProperties[propertyName] = true;

    if (ValidationProperties.All(pair => pair.Value))
        ValidationChangedEvent?.Invoke(this, true);

    return null;
}

And here's example of call which I want to be shorter:

 var result = Validate(nameof(FilePath),
                        new Tuple<Expression<Func<bool>>, string>(() => string.IsNullOrEmpty(FilePath), "Select a file to load"));

Basically I want to get rid of: new Tuple<Expression<Func<bool>>, string>

The class encapsulating the two properties suggested by @eurotrash is definitely a good idea. I think name ValidationRule should be ok.

class ValidationRule
{
    public ValidationRule(Func<bool> predicate, string message)
    {
        Predicate = predicate;
        Message = message;
    }
    public Func<bool> Predicate { get; }
    public string Message { get; }
}

I'd go further and create a collection so that I can use the collection initializer to specify the rules. This means collection class must implement the IEnumerable<T> interface and some Add(..) method. Then we can use the syntax:

new SomeCollection
{
   Item1, Item2
}

or if the Add(..) requires more then one parameter:

new SomeCollection
{
   { Item1, Item1a },
   { Item2, Item2a },
}

so our new collection is

class ValidationRuleCollection : IEnumerable<ValidationRule>
{
    private readonly List<ValidationRule> _rules = new List<ValidationRule>();

    public ValidationRuleCollection Add(Func<bool> predicate, string message)
    {
        _rules.Add(new ValidationRule(predicate, message));
        return this;
    }

    public IEnumerator<ValidationRule> GetEnumerator() => _rules.GetEnumerator();

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

Then you could use it like this

var result = Validate(nameof(FilePath), new ValidationRuleCollection
{
    { () => string.IsNullOrEmpty(FilePath), "Select a file to load" },
});

You don't need the Expression for it. You're not using it here. It adds overhead but does not have any advantage in this case.

The foreach loop can be replaced with LINQ. You stop at the first true result so use the FirstOrDefault to find it.

var rule = validationRules.FirstOrDefault(x => x.Predicate());
if (rule != null)
{
    ValidationProperties[propertyName] = false;
    ValidationChangedEvent?.Invoke(this, false);
    return rule.Message;
}


Java time-to-live collection Here's a class I made in java. It's pretty simple, the description of what it does is in the comment. I'd appreciate any feedback.

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

/**
 * An unordered, duplicate-allowing collection of items that are added with a
 * time-to-live, and will automatically be considered not a part of the
 * collection when that time expires. Has 0 thread overhead, and is not thread safe.
 */
public class TTLBag<E> implements Iterable<E> {

    private class Element {

        E item;
        long expires;

        Element(E item, long expires) {
            this.item = item;
            this.expires = expires;
        }

    }

    private List<Element> contents = new ArrayList<>();

    public void add(E item, long timeToLive) {
        contents.add(new Element(item, System.currentTimeMillis() + timeToLive));
    }

    @Override
    public Iterator<E> iterator() {
        long time = System.currentTimeMillis();
        contents.removeIf(element -> element.expires > time);
        return contents.stream().map(element -> element.item).iterator();
    }

}

You remove the items when the iterator is requested. Are you sure that this is right in your use case? Imagine the user puts an item in with a lifetime of 20 ms, requests the iterator, gets it with this one element and uses the iterator later. Theoretically, the item would be dead then. You should mention this behavior of the iterator method.

Additionally, the performance of the iterator construction depends on the number of dead entities inside the collection -> mention it.


Hash table for Coursera's algorithms course I implemented a hash table for Coursera's Algorithms and Design class. I am looking for feedback on coding style and other improvements. I want to practice C++ coding but I see myself drifting to C and functional programming a lot. Any advice on switching would be very helpful.

The problem is as follows:

  Given a list of 1 million numbers, find all the values of t in
  [-10000, 10000] such that if you pick 2 distinct numbers from the
  given list, they sum to t.

Solution (from one of the posts in the discussion forums):

  Store all the numbers in a hash table (map of (key, value) where
  key=int and value=vector). The hash function makes the key by dividing
  the number by 20000. Then, go to that key in the hash table and do a
  push_back on the vector for the value of that key.

Here are the 2 files:

#include<iostream>
#include <vector>
#include <fstream>
#include <set>
#include <map>
#include <cmath>
#include <sys/time.h>

using namespace std;

typedef set<long int> inner;
typedef map<int, inner> hashTable_T;

#include "ht2.h"

using namespace std;

void readfile(hashTable_T &H){
    ifstream ipstream;
    ipstream.open("algo1-programming_prob-2sum.txt");
    long int output; int index; hashTable_T::iterator it;
    while (ipstream >> output){
        index = output/(2*t);
        it = H.find(index);
        if (it==H.end()){
            inner innerVec;
            innerVec.insert(output);
            H.insert(make_pair(index,innerVec));
        }
        else{
            (it->second).insert(output);
        }
    }
    ipstream.close();
}

int findinTable(hashTable_T &H,const  long int &value, const int &binL,const int &binH, set<int> &setofTs){
    hashTable_T :: iterator it, itL, itH;
    inner :: iterator itIn;
    long int sum;

    itL = H.upper_bound(binL);
    if (itL!=H.begin()) itL--;
    itH = H.upper_bound(binH);

    bool done = false;
    while( itL!= H.end()){
        for (itIn = (itL->second).begin() ; itIn != (itL->second).end() ; itIn++){
            sum = *itIn + value;
            if (sum <= t && sum >= -t && *itIn!=value) setofTs.insert(sum) ;
        }
        itL++;
        if (done) break;
        if (itL == itH) done = true;
    }
}

void printHashTable(hashTable_T &H){
    hashTable_T::iterator it;
    inner::iterator vecIt;
    for (it=H.begin(); it!=H.end() ; it++){
        cout << it->first << " :";
        for (vecIt = (it->second).begin() ; vecIt != (it->second).end() ; vecIt++){
            cout << *vecIt << " ";
        }
        cout << endl;
    }
}

int main(){
    int t = 10000;
    hashTable_T HashTable;
    readfile(HashTable);

    set<int> setofTs;
    hashTable_T::iterator it = HashTable.begin();
    inner::iterator it2;
    int binL, binH;
    for (it;it!=HashTable.end();it++){
        for (it2 = (it->second).begin();it2 != (it->second).end(); it2++){
            binL = (-t-(*it2))/(2*t);
            binH = (t- (*it2))/(2*t);
            findinTable(HashTable, *it2, binL, binH, setofTs);
        }
    }
    cout << setofTs.size() << endl;
    return 0;
}

Well, if you're looking to move away from functional programming, making yourself some classes might be a good start.

In your .h file, you declare your class and all that jazz, so it becomes

#include <iostream>
#include <vector>
#include <fstream>
#include <set>
#include <map>
#include <cmath>
#include <sys/time.h>

using namespace std;

class HashTable
{
    typedef set<long int> inner;
    typedef map<int, inner> hashTable_T;
};

Only it doesn't. As has been pretty well established, using namespace std is bad practice. So this is what our header looks like now.

#include <iostream>
#include <vector>
#include <fstream>
#include <set>
#include <map>
#include <cmath>
#include <sys/time.h>

class HashTable
{
    typedef std::set<long int> inner;
    typedef std::map<int, inner> hashTable_T;

    hashTable_T hash;
};

Right, let's start having a look at that .cpp file. Now I see 3 functions that all do an operation on a single hash table. That smells functional to me, so I reckon we're actually going back to editing our .h. And since we're adding in some functions, may as well chuck in a little constructor.

As well as that, we need to think about visibility, as in setting what's public, protected and private, so we'll do that here too. Typically, the members of the class are either private or protected, so we'll make that happen, and whatever functions need to be accessed outside of the class are public, so we'll do that too.

We should give the variable int t, which is being passed to readFile(int) a meaningful name too, but I'm not sure what to call it, so I'll leave that up to you.

#include <iostream>
#include <vector>
#include <fstream>
#include <set>
#include <map>
#include <cmath>
#include <sys/time.h>

class HashTable
{
    typedef std::set<long int> inner;
    typedef std::map<int, inner> hashTable_T;

    private:
        hashTable_T hash;

    public:
        HashTable();
        void readFile(int t, const char* fileName);
        int findinTable(const long int &value, const int &binL,const int &binH, set<int> &setofTs);
        void print();
};

Okay, back to that .cpp.
We're killing that using namespace std again. I know all of the c++ tutorials use it, but trust me, you shouldn't.

Given that we've made it a class function, and we might want to read from other files (sorry to those shouting YAGNI, but this is just a little change), why not pass in a string which is the file name? As well as this, I've put in a few formatting changes which I think makes it a bit more readable. This means the readFile function becomes

void HashTable::readfile(int t, const char* fileName){

    ifstream ipstream(fileName);

    long int output;

    while (ipstream >> output){
        int  index = output/(2*t);
        hash[index].insert(output);
    }
}

After making it a class function, and doing a little bit of formatting, your findinTable is now:

int HashTable::findinTable(const long int &value, const int &binL, const int &binH, set<int> &setofTs){
    hashTable_T::iterator it, itL, itH;
    inner::iterator itIn;
    long int sum;

    itL = hash.upper_bound(binL);
    if (itL != hash.begin()){
        itL--;
    }

    itH = hash.upper_bound(binH);

    bool done = false;
    while(itL != hash.end()){
        for (itIn = (itL->second).begin(); itIn != (itL->second).end(); itIn++){
            sum = *itIn + value;
            if (sum <= t && sum >= -t && *itIn != value){
                setofTs.insert(sum);
            }
        }
        itL++;
        if (done){
            break;
        }
        if (itL == itH){
            done = true;
        }
    }
}

Getting onto your print function:

void HashTable::print(){
    hashTable_T::iterator it;
    inner::iterator vecIt;

    for (it=hash.begin(); it!=hash.end(); it++){
        std::cout << it->first << " :";
        for (vecIt = (it->second).begin(); vecIt != (it->second).end(); vecIt++){
            std::cout << *vecIt << " ";
        }
        std::cout << std::endl;
    }
}

Okay, looking pretty good so far. Probably not perfect, but a decent start. We wrote that constructor, so I think we should use it. I think it could probably use a bit of modification before it's worth writing though. So the declaration for it was HashTable();, but I think if it's made to be HashTable(const char* sourceFile, int tVal);, it'd be much better.

Then we'd have (as our constructor):

HashTable::HashTable(const char* sourceFile, int tVal){
    readFile(tVal, sourceFile);
}

Finally! Onto main. Now, I'm not entirely convinced about my solution to this but in particular, and I'm pretty sure that there's a more elegant way of doing it that I'm missing, but I'm going to say extract the bit which is doing the work, and put it into a class function. The only alternative (that I can see) is breaking the encapsulation, which is pretty non-OO. So I think that we add a function like this (but with a much better name):

std::set<int> HashTable::makeSet(int t){
    std::set<int> setofTs;
    hashTable_T::iterator it = hash.begin();
    inner::iterator it2;
    int binL, binH;
    for (it; it != hash.end(); it++){
        for (it2 = (it->second).begin(); it2 != (it->second).end(); it2++){
            binL = (-t-(*it2))/(2*t);
            binH = (t- (*it2))/(2*t);
            findinTable(*it2, binL, binH, setofTs);
        }
    }
    return setofTs;
}

So therefore main becomes:

int main(){
    int t = 10000;
    HashTable hTable("algo1-programming_prob-2sum.txt", t);

    std::set<int> setoFTs = hTable.makeSet(t);
    cout << setofTs.size() << endl;
    return 0;
}

And as a round-up, I'll show what states our 2 files are in at this point. I've done a bit of extra cleaning up that I'll explain as I go along in comments

ht2.h

#pragma once //Include guard, always good to have
#include <fstream> //Grouped the includes
#include <iostream>

#include <map>
#include <vector>
#include <set>

#include <cmath>
//Got rid of sys/time, as I couldn't see what it was being used for. If I'm wrong, ignore this.

class HashTable
{
    typedef std::set<long int> inner;
    typedef std::map<int, inner> hashTable_T;

    //Grouped all of the functions by type.
    private:
        hashTable_T hash;

    public:
        HashTable(){};
        HashTable(const char* sourceFile, int tVal);

        int findinTable(const long int &value, const int &binL,const int &binH, set<int> &setofTs);

        void print();
        void readFile(int t, const char* fileName);
};

ht2.cpp

#include "ht2.h"

HashTable::HashTable(const char* sourceFile, int tVal){
        readFile(tVal, sourceFile);
    }

void HashTable::readfile(int t, const char* fileName){

    ifstream ipstream(fileName);

    long int output;

    while (ipstream >> output){
        int index = output/(2*t);
        hash[index].insert(output);
    }
}

int HashTable::findinTable(const long int &value, const int &binL, const int &binH, set<int> &setofTs){
    hashTable_T::iterator it, itL, itH;
    inner::iterator itIn;
    long int sum;

    itL = hash.upper_bound(binL);
    if (itL != hash.begin()){
        itL--;
    }

    itH = hash.upper_bound(binH);

    bool done = false;
    while(itL != hash.end()){
        for (itIn = (itL->second).begin(); itIn != (itL->second).end(); itIn++){
            sum = *itIn + value;
            if (sum <= t && sum >= -t && *itIn != value){
                setofTs.insert(sum);
            }
        }
        itL++;
        if (done){
            break;
        }
        if (itL == itH){
            done = true;
        }
    }
}

std::set<int> HashTable::makeSet(int t){
    std::set<int> setofTs;
    hashTable_T::iterator it = hash.begin();
    inner::iterator it2;
    int binL, binH;
    for (it; it != hash.end(); it++){
        for (it2 = (it->second).begin(); it2 != (it->second).end(); it2++){
            binL = (-t-(*it2))/(2*t);
            binH = (t- (*it2))/(2*t);
            findinTable(*it2, binL, binH, setofTs);
        }
    }
    return setofTs;
}

void HashTable::print(){
    hashTable_T::iterator it;
    inner::iterator vecIt;

    for (it=hash.begin(); it!=hash.end(); it++){
        std::cout << it->first << " :";
        for (vecIt = (it->second).begin(); vecIt != (it->second).end(); vecIt++){
            std::cout << *vecIt << " ";
        }
        std::cout << std::endl;
    }
}

//This *could* be in a file all on it's own, but let's not make it look lonely.
int main(){
    int t = 10000;
    HashTable hTable("algo1-programming_prob-2sum.txt", t);

    std::set<int> setoFTs = hTable.makeSet(t);
    cout << setofTs.size() << endl;
    return 0;
}


Creating a class object just based on the class type I am trying to use a map as a way to implement an object factory. In Python it is possible to store a class type in a map, and use it later to create objects from that type:

class Foo:
    # ...

class Bar:
    # ...

factory_map = { 'foo' : Foo, 'bar' : Bar }
foo_object = factory_map['foo']()

I am not aware of a similar feature (i.e., storing a type in a map) in C++, so I came up with a solution based on lambda functions where the map stores functions responsible to create the objects:

class Base {
public:
    typedef std::unique_ptr<Base> Pointer;
    virtual void hi() = 0;
};

class Foo : public Base {
public:
    void hi() { std::cout << "Hi, it's foo\n"; }
};

class Bar : public Base {
public:
    void hi() { std::cout << "Hi, it's bar\n"; }
};

int main() {
    std::map<
        std::string,
        std::function<Base::Pointer()>> factory_map = {
            { "foo", []() { return Base::Pointer(new Foo()); } },
            { "bar", []() { return Base::Pointer(new Bar()); } },
        };

    Base::Pointer b1 = factory_map["foo"]();
    b1->hi();

    Base::Pointer b2 = factory_map["bar"]();
    b2->hi();
}

This might be the most straightforward solution to obtain Python's simplicity. I am wondering, however, if the usage of lambdas is the best way to create the factory. Specifically, I would like to know what are the pros/cons of this approach compared to a more "traditional" factory design (i.e., one using if/else conditionals to decide the type of the object to create).

The common argument for preferring one over the other for a dictionary (map) look up and a conditional (switch) look up is performance over maintainability.

The dictionary look up usually makes it easier to separate concerns. It also makes the code more concise and arguably more readable when invoking the factory functions.

The conditional look up avoids traversing a map and any types of indirection allowing for arguably faster access to object creation. It also avoids the overhead of creating the map/registering functions in the map.

There are also arguments for locality of reference vs. dependency management where the switch statement may keep object creation local to the call site, while the map approach passes off creation to a function or object that knows how to better handle the problem.

These are the typical arguments that I see for each of those different approaches to this problem.


Summing the prices from Transaction files I need to cut down the run time of this query. Currently it's taking 45 minutes. Is there something I can change in the table or the query to allow this to run faster?

SELECT SUM(B.PRICE),C.ST_FILE
FROM
    TAD1D.ST_EUCM AS B,
    TAD1D.ST_DSA_TRAN  AS C,
    TAD1A.TIBHLD T
WHERE C.ST_TRAN_FILE=B.ST_TRAN_FILE
    AND C.ST_IND='Y'
    AND C.ST_SRC_SYS_CD='HANN'
    AND C.ST_TBL_NAME='ST_EUCM'
    AND B.PARTNUMB=T.ACCT_NUM
    AND B.FVSRVCCODE=T.SRV_CD
GROUP BY C.ST_FILE
EXCEPT
SELECT sum(A.ACCUM_C),D.ST_FILE from
                    TAD1A.TIBHLD_DLY_VAL A,
                    TAD1D.ST_DSA_TRAN  AS D
            where A.SRC_SYS_CD = 'HANN'
                    AND D.ST_SRC_SYS_CD=A.SRC_SYS_CD
                    AND D.ST_TBL_NAME='ST_EUCM'
                    AND DATE(A.HLD_VAL_DT)=DATE(D.ST_FILE)
                    GROUP BY D.ST_FILE
                    WITH UR --

There are no indexes on either table. Please provide a tip/solution in the event i am able to change the table structure or add an index (I may not).

Nitpicks
There is a slight amount of inconsistency in your capitalization of keywords. SUM() / sum() and WHERE / where. Otherwise, it's pretty consistent throughout!

Aliases
You use the following aliases: B C T. There is no meaning or explanation of what those mean. You should use meaningful aliases, especially where your table names don't really help, such as in your case.

Old-style JOIN
This:
FROM
    TAD1D.ST_EUCM AS B,
    TAD1D.ST_DSA_TRAN  AS C,
    TAD1A.TIBHLD T
WHERE C.ST_TRAN_FILE=B.ST_TRAN_FILE
    AND C.ST_IND='Y'
    AND C.ST_SRC_SYS_CD='HANN'
    AND C.ST_TBL_NAME='ST_EUCM'
    AND B.PARTNUMB=T.ACCT_NUM
    AND B.FVSRVCCODE=T.SRV_CD

...is deprecated pre-ANSI-92 syntax. It should not be used, rather you should favor explicit JOIN syntax, like such:
FROM
    TAD1D.ST_EUCM AS B
INNER JOIN
    TAD1D.ST_DSA_TRAN  AS C
    ON C.ST_TRAN_FILE = B.ST_TRAN_FILE
INNER JOIN
    TAD1A.TIBHLD T
    ON B.PARTNUMB = T.ACCT_NUM
    AND B.FVSRVCCODE = T.SRV_CD
WHERE
    AND C.ST_IND = 'Y'
    AND C.ST_SRC_SYS_CD = 'HANN'
    AND C.ST_TBL_NAME = 'ST_EUCM'

Notice I added spaces around your = operators as it makes the code easier to read. You would also want to use similar JOIN syntax in your subquery.

Indexes
You stated:

There are no indexes on either table. Please provide a tip/solution in the event i am able to change the table structure or add an index (I may not).

And to that, I say resoundingly: YES! Add indexes!
But make sure even before you do that, look carefully at your query execution plan and look at where the most expensive steps are. That should give you a clue as to what is happening. I suspect there are a few nasty nested loops in there that are bogging it down, and depending on your RDBMS there are some optimizations available, and there may be ways to change the execution manually.


Rendering three types of events as HTML elements I have to clean up some Javascript and I am trying to figure out a more elegant way to write this code below. It's basically looping through 3 dictionaries with the same loop structure and appending some custom HTML onto a specific div.  I wanted to see if there is a way to avoid this repetitive code:

if (dtInt in calendarEventDict) {
    var arrEvents = calendarEventDict[dtInt];
    for (var a = 0; a < arrEvents.length; a++) {
        var calendarEvent = arrEvents[a];
        $("#dateCupcake" + i).append("
" + calendarEvent.DisplayName + "
");
    }
}
if (dtInt in vacationEvents) {
    var arrEventsV = vacationEvents[dtInt];
    for (var v = 0; v < arrEventsV.length; v++) {
        var vacationEvent = arrEventsV[v];
        $("#dateCupcake" + i).append("
" + vacationEvent.PersonName + "
");
    }
}
if (dtInt in travelEvents) {
    var arrEventsV1 = travelEvents[dtInt];
    for (var v1 = 0; v1 < arrEventsV1.length; v1++) {
        var travelEvent = arrEventsV1[v1];
        $("#dateCupcake" + i).append("
" + travelEvent.TravelRequest.Person.LastName + ", " + travelEvent.TravelRequest.Person.FirstName.Substring(0, 1) + " visiting " + travelEvent.TravelRequest.TechnicalCentre.Name + "
 ");
    }
}

The simplest refactor would be to simply pull out the common functionality, and pass in the array for each case, as well as a function used to create your div content from the current item in the loop.  Assuming dtInt is global:

function checkDtInt(arr, funcToCreateDiv) {
    if (dtInt in arr) {
        var arrEventsV1 = arr[dtInt];
        for (var v1 = 0; v1 < arrEventsV1.length; v1++) {
            var travelEvent = arrEventsV1[v1];
            $("#dateCupcake" + i).append(funcToCreateDiv(travelEvent));
        }
    }
 }

checkDtInt(calendarEventDict, function(item) {
   return "
" + item.DisplayName + "
"
});

checkDtInt(vacationEvents, function(item) {
   return "
" + item.PersonName + "
"
});

checkDtInt(travelEvents, function(item) {
   return "
" + item.TravelRequest.Person.LastName + ", " + item.TravelRequest.Person.FirstName.Substring(0, 1) + " visiting " + item.TravelRequest.TechnicalCentre.Name + "
 "
});


Animating based on screen width Depending on the width of the screen the JavaScript animates a node and then loops another animation.

I'm trying to understand DRY but my JavaScript skills aren't quite up to scratch to get this code as optimised as possible. I'm also wanting to implement $(window).resize(function() { }); to detect this change without a refresh. Is using constants the best way to optimise this code? If so, how would this be achieved?

jsFiddle

'use strict';

/*jslint browser: true*/
/*global $, jQuery*/

/*
--------------------------------------------------------
VARIABLES
--------------------------------------------------------
*/

var width       = $(window).width(),        // window width
    down_dur,
    left_dur,
    up_dur,
    right_dur;

/*
--------------------------------------------------------
FUNCTION CODE TO BE EXECUTED
--------------------------------------------------------
*/

function animate_node(down, down_dur, left, left_dur, up, up_dur, right, right_dur) {

    down        = down  + 'px';
    left        = left  + 'px';
    up          = up    + 'px';
    right       = right + 'px';

    $('.node')
        .animate({top: down},
            { duration: down_dur, easing : 'linear', queue: true })
        .animate({marginLeft: left},
            { duration: left_dur, easing : 'linear', queue: true })
        .animate({top: up},
            { duration: up_dur, easing : 'linear', queue: true })
        .animate({marginLeft: right},
            { duration: right_dur, easing : 'linear', queue: true });
}

/*
--------------------------------------------------------
CALL THE FUNCTIONS
--------------------------------------------------------
*/

// fire the following when the dom is ready
$(function () {

    if (width > 840) {
        $('.node').each(function(i) {
            $(this).delay(1500 * (i + 1))
                .animate({top: '157px'},
                    { duration: 1000, easing : 'linear', queue: true })
                .animate({marginLeft: '264px'},
                    { duration: 1000, easing : 'linear', queue: true });
        });
        animate_node(425, 2000, -284, 2000, 157, 2000, 264, 2000);
        setInterval(function() {
            animate_node(425, 2000, -284, 2000, 157, 2000, 264, 2000);
        }, 2000);
    } else if (width < 840 && width > 600) {
        $('.node').each(function(i) {
            $(this).delay(1500 * (i + 1))
                .animate({top: '165px'},
                    { duration: 1000, easing : 'linear', queue: true })
                .animate({marginLeft: '179px'},
                    { duration: 1000, easing : 'linear', queue: true });
        });
        animate_node(490, 2000, -199, 2000, 165, 2000, 179, 2000);
        setInterval(function() {
            animate_node(490, 2000, -199, 2000, 165, 2000, 179, 2000);
        }, 2000);
    } else if ((!(document.documentElement.hasOwnProperty('ontouchstart'))) &&
            width < 600) {
        $('.node').each(function(i) {
            $(this).delay(1833.333 * (i + 1));
        });
        animate_node(1095, 5000, 102, 500, 165, 5000, -9, 500);
        setInterval(function() {
            animate_node(1095, 5000, 102, 500, 165, 5000, -9, 500);
        }, 2000);
    }

});

I would move all positions and other configuration to a simpleObject which contains a condition which is checked to determine if the configuration should be used for the current screen dimensions:

Remove unneeded variables
Added functions to remove duplication
Moved positions to animationConfig so logic and configuration is separated

I made a assumption in the code which is that the path is always a square, so up, down, left
and right duration are replaced with a horizontal and vertical duration

jsfiddle: http://jsfiddle.net/EJF9q/1/

'use strict';

function animate_node(down, down_dur, left, left_dur, up, up_dur, right, right_dur) {

    function animate_cfg(duration) {
        return { duration: duration, easing : 'linear', queue: true };
    }

    down    += 'px';
    left    += 'px';
    up      += 'px';
    right   += 'px';

    $('.node')
        .animate({top: down},           animate_cfg(down_dur))
        .animate({marginLeft: left},    animate_cfg(left_dur))
        .animate({top: up},             animate_cfg(up_dur))
        .animate({marginLeft: right},   animate_cfg(right_dur));
}

function animate_config(config) {
    animate_node(
        config.down,
        config.vertical_dur,
        config.left,
        config.horizontal_dur,
        config.up,
        config.vertical_dur,
        config.right,
        config.horizontal_dur);
}

// OnReady
$(function () {

    var animationConfigs = [
        {
            startTop: 157,
            delay: 1500,
            down: 425,
            left: -284,
            up: 157,
            right: 264,
            vertical_dur: 2000,
            horizontal_dur: 2000,
            conditions: function() {
                return $(window).width() > 840;
            }
        },
        {
            delay: 1500,
            down: 490,
            left: -199,
            up: 165,
            right: 179,
            vertical_dur: 2000,
            horizontal_dur: 2000,
            conditions: function() {
                return $(window).width() < 840 && $(window).width() > 600;
            }
        },
        // Etc...
    ];

    for (var i in animationConfigs) {
        var animationCfg = animationConfigs[i];
        if (animationCfg.conditions()) {
            $('.node').each(function(i) {
                $(this).delay(1500 * (i + 1))
                    .animate({ top: animationCfg.up + 'px' },
                        { duration: 1000, easing : 'linear', queue: true })
                    .animate({marginLeft: animationCfg.right + 'px'},
                        { duration: 1000, easing : 'linear', queue: true });
            });
            animate_config(animationCfg);
            setInterval(function() {
                animate_config(animationCfg);
            }, 2000);
            break;
        }
    }
});


When push data to array decide which key need to use Sometimes I need to push data to the array, but I have one different key (here I have key color and key name) and another part of the object is the same. I use IF/else to check this, but i wonder is there any better and cleanest way to handle this?
  getData(passedData, dataControl: string) {
    const data = [];

    for (const value of passedData) {
      if (dataControl === 'color') {
        data.push({
          color: value,   ---> diff
          username: this.userForm.value.username,
          test: this.userForm.value.test
        }
        );
        this.userForm.controls['data1'].setValue(data);     ---> diff

      } else {
        data.push({
          name: value,     ---> diff
          username: this.userForm.value.username,
          test: this.userForm.value.test
        }
        );
        this.userForm.controls['data2'].setValue(data);       ---> diff

      }
    }
  }

Thank you

A few thoughts:

First, you might consider whether the name/color property REALLY needs to change as output here.  Why return { color: ... } in some cases and { name: ... } in others?  This tends to indicate that the return data may not sufficiently be describing itself. Even stranger here is the fact that the caller must pass in the hint dataControl "hint" in order to tell the function to modify the return value.  If the calling code is where understanding of "color" vs. "name" exists, then why make this function now have to understand this?  Why can the caller not just mutate a returned { name: ... } object into a { color: ... } one if it is really critical to make this distinction in property name.  Perhaps your object might be well suited to have properties like username, test, value (instead of name or color) and type (to indicate name vs. color).  At least this way to get a consistently formed data objects here (and your if-else goes away).
I am struggling to understand why you are looping through the passed data and setting data1 or data2 value repeatedly when, you are basically overwriting this set value X times (where X is the length of the array).  Why not just build the array once and then set the value?
You really only have two things that are changing when dataControl equals color.  You can just use variables and drop the if-else construct altogether.  In fact generally-speaking, much of you logic can be put into variables, as these values do not change for each iteration over passed data. Using variables here will also simplify your loop such that perhaps all you really need to do is call map on passedData.
Why hard-code your data1 and data2 targets?
It looks like this code has TypeScript type hinting, however no TypeScript tag was applied to this question.  You should either be consistent with your TypeScript usage (putting type hint for BOTH parameters), or not use it it all.

Putting it all together, might yield something like...
const getData = (passedData, dataControl) {
  let propName = 'name';
  let target = 'data2';

  if (dataControl === 'color') {
    propName = 'color';
    target = 'data1';
  }

  const username = this.userForm.value.username;
  const test = this.userForm.value.test;
  const targetEl = this.userForm.controls[target];

  const data = passedData.map((value) => {
    return {
      // if you still REALLY need color vs. name prop
      [propName]: value,
      username,
      test
    };
  });

  targetEl.setValue(data);
}


My first project using namespaces I have to create some APIs for a online game and I decided to learn to use namespaces correctly.

I used the factory pattern, then the loadModule method checks if in the Modules folder there is a class named $module and then it return the object.
The folders are synchronized with the namespaces.

The main class Metin2.class.php:

<?php

namespace Metin2;
use mysqli;
use \Metin2\Utilities as Utilities;
require_once("config.php");
require_once(UTILITIES_PATH . "Autoloader.class.php");

class Metin2
{
    public $db;

    public function __construct(mysqli $database)
    {
        spl_autoload_register("Metin2\\Utilities\\Autoloader::loadModel");
        try {
            if(!$database) throw new Exceptions\Metin2Exception("The database instance     is not a valid instance");
            $this->db = new Utilities\Database($database);

        } catch (Exceptions\Metin2Exception $e) {
            echo $e->errorMessage();
        }

    }

    public function loadModule($module)
    {
        $m = "\\Metin2\\Modules\\" . $module;
        return new $m;
    }
}

Autoloader class:

<?php
namespace Metin2\Utilities;

class Autoloader {
    public static function loadModel($class){
        $class = str_replace("\\","/",$class);
        $class = explode("/",$class);
        if($class[0] == "Metin2")unset($class[0]);
        $class = implode("/",$class);
        $path = ROOT_PATH.$class.".class.php";
        if(file_exists($path)){
            require_once($path);
        }
    }
}

An example of a module:

<?php
namespace Metin2\Modules;

class Testdb {

    public $db;
    public function __construct(){
        $this->db = \Metin2\Utilities\Database::getInstance();
        print_r($this->db->getOne("account"));
    }
}

Everything works well, but I want to know if there are any things I did wrong or if there are things I can do better.

My thoughts:

Imho Database should not belong to Utilities namespace. Working with database is likely a core part of your application, "Utilities" suggest auxiliary tools to make your life easier. But maybe the class does something different than I expect
Is there a reason to keep all your application in Metin2 namespace but have separate Exceptions namespace with Metin2Exception? If it's for code completition, then a good IDE should only suggest classes inheriting from Exception in these contexts.
Metin2::__constructor(): can Utilities\Database throw Metin2Exception? If not, then it's not really useful to throw exception only to catch it right away and echo
It's worth considering using specialized Exception classes and keep Exception messages for details. That is because you can't catch Exception based on its message, but only class. Here you could have ConnectionFailedException extends Metin2Exception.
Instead of echoing Exception message I would recommend making a function/class, that would enable you to redirect error messages to file, as it's not recommended to be printing out application messages for users to see. This way you'll also be able to log them.
On a similar note you may take a look at set_exception_handler, that lets you assign a function/method to handle uncaught exceptions. And from that you can try either making your own or using a 3rd party exception handlers, which combined with set_error_handler that can be used for conversion of errors to exceptions can really improve your programming. Examples: https://github.com/nette/tracy or ... maybe http://raveren.github.io/kint/ (I'm sure there's another as nice as Tracy, but I can't remember its name).
You can remove dependency of Autoloader on a global constant by adding a static property and using a setter to set its value
When Autoloader finds out the target filepath does not exist, it does not react in any way ... maybe at least log it somehow, if not throw exception?
What's the purpose of Testdb? I would discourage from using a service location especially when it comes to testing ... why not pass the $db as an argument?


Does my Linklist implementation have memory leaks? Are there any memory leaks in this linklist implementation.Also is the implementation correct?Can the time complexity be optimized?

#include <iostream>
#include <cstdlib>
#include <cstdio>
using namespace std;

struct node
{
int n;
node* point;
};

node* first=NULL;

void search(int n);
void insert(int n);
void del(int n);
void print();

void menu()
{
int s,n;
cout<<"Enter 1 to Search"<<endl;
cout<<"Enter 2 to Insert"<<endl;
cout<<"Enter 3 to Delete"<<endl;
cout<<"Enter 4 to Print"<<endl;
cout<<"Enter 5 to Exit"<<endl;

cin>>s;

switch(s)
{
    case 1: cout<<"Enter number to Search: ";
            cin>>n;
            search(n);
            break;

    case 2: cout<<"Enter number to Insert: ";
            cin>>n;
            insert(n);
            break;

    case 3: cout<<"Enter number to Delete: ";
            cin>>n;
            del(n);
            break;

    case 4: print();
            break;

    case 5: exit(0);

    default: cout<<"Wrong Input"<<endl<<endl<<endl;
}
}

int main()
{
cout<<"Welcome to Linked List"<<endl;
while(1)
    menu();
}

void search(int n)
{
node* ptr=first;

while(ptr!=NULL)
{
    if(ptr->n==n)
    {
        cout<<endl<<n<<" Found!"<<endl<<endl<<endl;
        return;
    }
    else
        ptr=ptr->point;

}

cout<<endl<<n<<" Not Found!"<<endl<<endl<<endl;
return;
}

void insert(int n)
{
node* prev=NULL;
node* next=first;

if(first!=NULL and first->n>n)
{
    first=new node;
    first->n=n;
    first->point=next;
    cout<<endl<<endl;
    return;
}

while(next!=NULL)
{
    if(next->n>n)
    {
        prev->point=new node;
        (prev->point)->n=n;
        (prev->point)->point=next;
        cout<<endl<<endl;
        return;
    }
    else
    {
        prev=next;
        next=next->point;
    }
}
    if(prev==NULL)
    {
        first=new node;
        first->n=n;
        first->point=NULL;
    }
    else
    {
        prev->point=new node;
        (prev->point)->n=n;
        (prev->point)->point=NULL;

    }
    cout<<endl<<endl;
    return;

}

void print()
{
node* next=first;

while(next!=NULL)
{
    cout<<next->n<<" ";
    next=next->point;
}
cout<<endl<<endl<<endl;
return;
}

void del(int n)
{
node* prev=NULL;
node* next=first;

while(next!=NULL)
{
    if(next->n==n)
    {
        if(prev!=NULL)
        {
            prev->point=next->point;
            delete next;
        }
        else
        {
            first=next->point;
            delete next;
        }
        cout<<n<<" Deleted!";
        cout<<endl<<endl;
        return;
    }
    else
    {
        prev=next;
        next=next->point;
    }
}

cout<<"Number not in list"<<endl<<endl<<endl;
return;
}

It does not appear to be leaking memory (except on exit). However, there are a number of things you can do to improve this code.

Fix the formatting

Code that has poor formatting is hard to read, understand, and maintain.  For that reason, you should strive to have nicely formatted code.  In this case, that means fixing indentation and inserting whitespace.  For instance, instead of this:

cout<<"Number not in list"<<endl<<endl<<endl;

write this:

cout << "Number not in list" << endl << endl << endl;

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Separate I/O from core routines

It's often better to have input and output operation in a single place (or a few select places) rather than having them scattered in several places.  For this reason, I'd recommend having the search, insert and del routines simply be responsible for their corresponding linked list function and not print anything.  Printing can be done instead in the calling routine if needed.

Use object orientation

Because you're writing in C++, it would make sense to have the methods that operate on a class (such as node) be member functions rather than separate functions.  Objects and classes are one of the main strengths of C++ and something you should learn soon if you haven't already.  Encapsulate related code in objects where it makes sense to do so.

Eliminate global variables

Eliminating global variables will make your code more readable and maintainable, both of which are important characteristics of well-written code.  Global variables introduce messy linkages that are difficult to spot and error-prone.  Global variables can be altered by literally any code, making it difficult to understand how they're being used, and making it all too easy to inadvertently create an inconsistent state. See "how bad are global variables?" for more. The variable first should instead be passed explicitly to each routine that needs it, or you can use an object as mentioned in the previous suggestion.

Return something useful from functions

All of your functions currently return void, but the code could be improved by making some of them return a bool indicating success or failure.  For instance, del could return false if the number is not in the list or true if it was found and deleted.

Use nullptr rather than NULL

Modern C++ uses nullptr rather than NULL.  See this answer for why and how it's useful.

Use for instead of while where appropriate

In almost every place in the code that uses a while loop, it would be much more idiomatic C++ if a for loop were used instead. For example, the print routine is currently this:

void print()
{
    node *next = first;

    while (next != NULL) {
        cout << next->n << " ";
        next = next->point;
    }
    cout << endl << endl << endl;
    return;
}

It would be better written like this:

void print(node *head)
{
    for ( ; head != nullptr; head = head->point) {
        std::cout << head->n << " ";
    }
    std::cout << "\n\n\n";
}

Note that the head parameter is being passed in rather than using a global variable, and that the namespace is explicitly used. Also, note that I've used the string literal "\n\n\n" instead of three repetitions of std::endl. This is more efficient because it doesn't force the buffer to be flushed repeatedly. Finally, I've removed the return because it is not needed for a void function.

Rethink the interface

It would be simpler and more logical for the search routine to return a pointer to the node or nullptr if it is not found.  Here's one way to write it:

node* search(node *head, int n)
{
    for ( ; head != nullptr && head->n != n; head = head->point ) {
    }
    return head;
}

Sanitize user input

Right now, if the user inputs anything that is not a number, the program stays in a loop forever. Instead of extracting an int, get a std::string and use std::stoi() to convert it to an integer.

Use helper routines

Right now, there is a lot of code duplication.  The search, insert and delete routines all search for the node first.  If you extracted this code into a helper routine to find the node before a match, returning a node *, you could reuse searchPrev within insert and delete instead of duplicating it in every function:

node* searchPrev(node *head, int n)
{
    for ( ; head && head->point && head->point->n < n; head = head->point ) {
    }
    return head;
}


Python: Function to Find 2 Closest Points Among [x,y] Coordinates I'm writing some code in Python that takes a set of [x,y] coordinates (in a list of tuples) and also an x-value. I need to find the two closest x coordinates to my x-value.

To find the closest x, I created a list of all the x-coordinates, used that list to create a lambda function that found the closest x to my x-value, and then found the index of that x.

Finding the 2nd closest value was a bit trickier. I created a new list that excluded the closest x and followed the same process.

Code is below. Any suggestions on how to improve this or make it more efficient?

def closest_2_points(list_of_tuples, x_value):

    x_points = []

    for i in list_of_tuples:
        x_points.append(i[0])

    # find closest point
    closest_xpoint_1 = min(x_points, key=lambda z:abs(z-x_value))
    closest_xpoint_1_idx = x_points.index(closest_xpoint_1)

    # find 2nd closest point
    x_points_2 = [x for x in x_points if x != closest_xpoint_1]
    closest_xpoint_2 = min(x_points_2, key=lambda z:abs(z-x_value))
    closest_xpoint_2_idx = x_points.index(closest_xpoint_2)

    closest_points = [(list_of_tuples[closest_xpoint_1_idx][0], list_of_tuples[closest_xpoint_1_idx][1]), (list_of_tuples[closest_xpoint_2_idx][0], list_of_tuples[closest_xpoint_2_idx][1]) ]

    return closest_points

When running the function, it should look something like this:

closest_2_points([(4,6),(2,5),(0,4),(-2,3)], 6)

And return something like this:

[(4, 6), (2, 5)]

If there are duplicate points in your list, and the closest point turns out to be one of these points, this line of code will remove all of the duplicates:
x_points_2 = [x for x in x_points if x != closest_xpoint_1]

So, instead of returning the 2 closest points, it may return just one of the closest, and some other nearby point.

Finding the n-smallest, n-closest, etc is usually best done using Python's heapq.nsmallest(n, iterable, key=None) function.

Return a list with the n smallest elements from the dataset defined by iterable. key, if provided, specifies a function of one argument that is used to extract a comparison key from each element in iterable (for example, key=str.lower). Equivalent to: sorted(iterable, key=key)[:n].
[This function] perform best for smaller values of n. For larger values, it is more efficient to use the sorted() function. Also, when n==1, it is more efficient to use the built-in min() and max() functions. If repeated usage of these functions is required, consider turning the iterable into an actual heap.

Example: a closest n points function, with n defaulting to 2:
import heapq

def closest_points(list_of_tuples, x_value, n=2):
    return heapq.nsmallest(n, list_of_tuples, lambda pnt:abs(pnt[0] - x_value))

Example:
>>> closest_points([(4, 6), (2, 5), (0, 4), (-2, 3)], 6)
[(4, 6), (2, 5)]


Computing sums of weird pattern in 2D array as efficiently as possible This is a problem from Hackerrank (https://www.hackerrank.com/challenges/2d-array/problem). We're given a 6x6 (always) 2D array and asked to compute the sums of all the hourglass patterns in the array. An hourglass pattern is of the shape
1 1 1
  1
1 1 1

where the 1's form the hourglass. In this case the sum is 7, but it could be any integer from -63 to 63, the constraints being: -9 <= arr[i][j] <= 9. There are 16 hourglasses in each 6x6 2D array, and we're asked to return the greatest hourglass value.
As an example, the following 2D array has a maximum hourglass value of 28:
-9 -9 -9  1 1 1
 0 -9  0  4 3 2
-9 -9 -9  1 2 3
 0  0  8  6 6 0
 0  0  0 -2 0 0
 0  0  1  2 4 0

My code:
def hourglassSum(arr):
    max_hourglass = -63
    for column in range(len(arr)-2):
        for row in range(len(arr)-2):
            max_hourglass = max(arr[row][column] + arr[row][column+1] + arr[row][column+2] \
            + arr[row+1][column+1] + arr[row+2][column] + arr[row+2][column+1] \
                                + arr[row+2][column+2], max_hourglass)
    return max_hourglass

Is there any way to make this faster / more efficient? I'm reusing a lot of the same numbers in my calculations, which seems wasteful; is there a dynamic programming solution I'm not seeing, anything else? I appreciate any comments or optimization opportunities on my code, thank you.

Drat. My super pretty NumPy solution which is probably efficient doesn't get accepted because HackerRank apparently doesn't support NumPy here. Oh well, here it is anyway, maybe interesting/amusing for someone.
import sys
import numpy as np

a = np.loadtxt(sys.stdin, dtype=np.int8)

h = a[0:4, 0:4] + a[0:4, 1:5] + a[0:4, 2:6]     + \
                  a[1:5, 1:5]                   + \
    a[2:6, 0:4] + a[2:6, 1:5] + a[2:6, 2:6]

print(h.max())

Or with less code repetition:
import sys
import numpy as np

a = np.loadtxt(sys.stdin, dtype=np.int8)

i, j, k = (slice(i, i+4) for i in range(3))

h = a[i,i] + a[i,j] + a[i,k]     + \
             a[j,j]              + \
    a[k,i] + a[k,j] + a[k,k]

print(h.max())

The 4x4 submatrix a[0:4, 0:4] contains the top-left value for each of the 16 hourglasses, a[0:4, 1:5] contains the top-middle value for each of the 16 hourglasses, etc. So adding them computes a 4x4 matrix containing the 16 hourglass sums.


File mover based on document name I wrote a script that places documents to the folders they should be (based on my preferences of course).
It even creates the folder if it doesn't exists.
Here's how it works:

You run de script by clicking on it. (idk if I should let this run in the background constantly, I don't use it that much)
Checks the selected directory for files with a "-" in their name.
(Physics- Homework.docx)
It checks the targeted directory (where my folder hierarchy is meant to be) if there's a folder named after the file's name before the "-" part. ("Physics- Homework.docx" goes to a folder named "Physics")
If there isn't a folder, it creates one. If there is a folder, it just moves the file inside

So Like I said in the first step, you have to click to run this, I would love to make it automatic, but I don't want it to run in the background just to move a file twice in a week or so. I would love to know if there's another cool way to achieve this automatic way of working.
My idea (to achieve what I said above) was to create a ".bat" file that runs Word and the script at the same time (this time script keeps running in the background once started), next to script's based functionality, it would also check for the apps running in the background and when Word is closed, it performs the file movement and closes itself. But I don't know if this is a great idea or if it can be done in much simpler way.
I want to know what I could've done better. Since the code is relatively short, I feel like I have done a great job, but I believe that there's always room for improvement.
Here's the code:
import shutil
import os

Dir = "D:/OneDrive/Documents/"  # Where are the files? Add "/" at the end of your directory.
TragetDir = "D:/OneDrive/Documents/school/6a9/"  # Where are the target folders? Add "/" at the end of your directory.

FileNames = glob.glob(Dir + "*-*")  # scans the dir with the selected character (*[selected character]*), searches directly for the name now.

StartIndex = len(Dir)  # The chars of the Dir are useless
for x in range(len(FileNames)):  # Each file gets moved one by one
    Index = FileNames[x].find("-")  # I always create my files using: "class + "-" + topic" way
    SFolder = FileNames[x][StartIndex:Index]  # My folders are always named after the subject, so "Math- exam 1.docx" should be placed in a folder named "Math"

    try: #tries to make a target dir
        os.makedirs(TragetDir + SFolder)
    except: #if the directory already exists just skips this process
        pass

    shutil.move(FileNames[x], TragetDir + SFolder)  # moves to the target folder

A few changes I would make:

Follow PEP 8.
Use the modern, high-level pathlib instead of a combination of the os module, glob, and shutil.
Be careful when using a bare except:, or except Exception:, see https://stackoverflow.com/q/54948548.

import pathlib

docs_dir = pathlib.Path('')
target_dir = pathlib.Path('')

for file in docs_dir.glob('*-*'):  # Each file gets moved one by one
    subject_name, file_name = str(file.name).split('-', maxsplit=1)
    subject_dir = target_dir.joinpath(subject_name)

    subject_dir.mkdir(exist_ok=True)
    file.rename(subject_dir.joinpath(file_name))

As an aside, it may be worth considering using a folder just for unsorted school-related files. It would be so easy to add a non-schoolwork file which contains a '-' to your documents folder, and have it moved unintentionally by the program.
You wrote in a comment:

However I aim for that "I saved the file and it directly moved to the targeted folder" approach.

If that's the case, you should look into file system watchers/watchdogs.


Random number guessing Is there anything specifically bad here, or anything that I could improve? Wasted lines of code, things done in a more memory intensive way than it could be, etc.

from random import*
b = 10
a = randint(1,b)
point = 1
x = 1
while x < 2:
    print("Guess a number between 1 and ", b)
    svar = int (input())
    if svar == a:
            b+=5
            point= point+point
            a = randint (1,b)
            print("You have ", point, "points!")
    elif svar < a:
            print("Higher")
    else:
            print("Lower")

Style

The general standard for Python is to indent by four spaces, or by one tab with a width of four spaces.
You shouldn't be doing wildcard imports, (from a import *).
Your naming can be greatly improved. For example, the variable b could be renamed to maximum_guess_limit.
Use str.format to format strings instead of using commas between string literals. Here's an example:

# str.format without positional or named parameters
print("{} {}".format("Hello", "world"))

# str.format with positional parameters
print("{1} {0}".format("world", "Hello"))

# str.format with named parameters
print("{word1} {word2}".format(word1="Hello", word2="world"))

Design

The logic in this program is very confusing, and with the bad variable names, it's not any easier to understand. From looking at the code, it looks like a number-guessing game of sorts, but I don't fully understand it's logic. I'd recommend encapsulating this into a function, or even separate functions. For example, to choose a random number based on a maximum integer, I'd create a function that looks something like this:

def choose_random_value(maximum_guess_limit):
    return randint(1, maximum_guess_limit)

In the end, I ended up with a result like this:

from random import randint

class NumberGuessingGame:
    def __init__(self, maximum_guess_limit):
        self.maximum_guess_limit = maximum_guess_limit
        self.__points = 0

    def __choose_random_integer(self):
        """
        Choose a random integer between one and the
        current maximum limit.
        """
        return randint(1, self.maximum_guess_limit)

    def __obtain_user_input(self, prompt):
        """
        Obtain the user input and ensure that
        it is a valid integer.
        """
        try:
            return int(input(prompt))
        except ValueError:
            print("You must enter a valid integer.")
            return None

    def __check_user_input(self, user_input, random_integer):
        """
        Check if the user guessed correctly, otherwise,
        display "Higher." or "Lower."
        """
        if user_input is not None:
            if user_input == random_integer:
                self.maximum_guess_limit += 5
                self.__points += self.__points

            elif user_input < random_integer:
                print("Higher.")

            else:
                print("Lower.")

    def play_game(self):
        while True:
            user_input = self.__obtain_user_input("Enter an integer: ")
            integer_to_guess = self.__choose_random_integer()
            self.__check_user_input(user_input, integer_to_guess)

game = NumberGuessingGame(10)
game.play_game()

It doesn't have support for exiting, but I'm sure you can implement it.


Calculate Cosine similarity between Multisets Having read "Effective Java" and "Clean Code" I wanted to apply these principles in a manageable scenario. To this end I refactored a Java library that calculates similarities between strings.

The code below is that of a cosine metric. Documents are tokenized into multisets of strings. These are passed onto the metric which calculates their magnitude and dot product by treating the multisets as if they were (sparse) vectors.

package org.simmetrics.metrics;

import static com.google.common.collect.Multisets.union;
import static java.lang.Math.sqrt;

import org.simmetrics.MultisetMetric;

import com.google.common.collect.Multiset;

/**
 * Calculates the cosine similarity over two multisets. The similarity is
 * defined as the cosine of the angle between the multisets expressed as sparse
 * vectors.
 *

 *
 * similarity(a,b) = a·b / (||a|| * ||b||)
 *
 *
 *

 * The cosine similarity is identical to the Tanimoto coefficient, but unlike
 * Tanimoto the occurrence (cardinality) of an entry is taken into account. E.g.
 * {@code [hello, world]} and {@code [hello, world, hello, world]} would be
 * identical when compared with Tanimoto but are dissimilar when the cosine
 * similarity is used.
 *

 * This class is immutable and thread-safe.
 *
 * @see TanimotoCoefficient
 * @see Wikipedia
 *      Cosine similarity
 *
 * @param <T>
 *            type of the token
 */
public final class CosineSimilarity<T> implements MultisetMetric<T> {

    @Override
    public float compare(Multiset<T> a, Multiset<T> b) {

        if (a.isEmpty() && b.isEmpty()) {
            return 1.0f;
        }

        if (a.isEmpty() || b.isEmpty()) {
            return 0.0f;
        }

        float dotProduct = 0;
        float magnitudeA = 0;
        float magnitudeB = 0;

        // Lager set first for performance improvement.
        // See: MultisetUnionSize benchmark
        if(a.size() < b.size()){
            final Multiset<T> swap = a; a = b; b = swap;
        }

        for (T entry : union(a, b).elementSet()) {
            float aCount = a.count(entry);
            float bCount = b.count(entry);

            dotProduct += aCount * bCount;
            magnitudeA += aCount * aCount;
            magnitudeB += bCount * bCount;
        }

        //  a·b / (||a|| * ||b||)
        return (float) (dotProduct / (sqrt(magnitudeA) * sqrt(magnitudeB)));
    }

    @Override
    public String toString() {
        return "CosineSimilarity";
    }

}

This is pretty good, but two things come immediately to mind:

Declare variables closest to where they're being used. Declare dotProduct, magnitudeA, and magnitudeB right above your loop.
Don't reassign method arguments. That's really confusing six months down the road. In this case, use a helper method which takes the larger and smaller sets in a specific order.

Example:

import static java.lang.Math.sqrt;

/**
 * [Excluded for brevity]
 *
 * @param <T>
 * type of the token
 */
public final class CosineSimilarity<T> implements MultisetMetric<T> {

    @Override
    public float compare(final Multiset<T> a, final Multiset<T> b) {

        if (a.isEmpty() && b.isEmpty()) {
            return 1.0f;
        }

        if (a.isEmpty() || b.isEmpty()) {
            return 0.0f;
        }

        if (a.size() >= b.size()) {
            return this.determineSimilarity(a, b);
        } else {
            return this.determineSimilarity(b, a);
        }
    }

    private float determineSimilarity(final Multiset<T> largerSet, final Multiset<T> smallerSet) {
        float dotProduct = 0;
        float magnitudeA = 0;
        float magnitudeB = 0;

        for (final T entry : union(largerSet, smallerSet).elementSet()) {
            final float aCount = largerSet.count(entry);
            final float bCount = smallerSet.count(entry);

            dotProduct += aCount * bCount;
            magnitudeA += aCount * aCount;
            magnitudeB += bCount * bCount;
        }

        // a·b / (||a|| * ||b||)
        return (float) (dotProduct / (sqrt(magnitudeA) * sqrt(magnitudeB)));
    }

    @Override
    public String toString() {
        return "CosineSimilarity";
    }
}


Nesting elements with appendChild I want to make a nav that look like this:

Using the appendChild() method, and this is what I've done:

The myNode is the nav tag which has already been created via HTML.

var myNode = document.querySelector('.socialmediaicons');

var myEl = document.createElement('ul');

var myLi = document.createElement('li');

var myA = document.createElement('a');

var myImg = document.createElement('img');

var addEl = function() {
    var nodes = [myEl, myLi, myA, myImg];

    for(var i=0; i<nodes.length; i++) {
        myNode.appendChild(nodes[i]);

        for(var j=0; j<3; j++){
            nodes[j].appendChild(nodes[j+1]);
        }
    }

};

addEl();
console.log(myNode);

And it WORKS!!

I even added a src attribute to the myImg element using myImg.src= and the photo shows up. But is this code GOOD!?

Is there any better way to do it? (If you have a jQuery method it's OK, but show me the pure JavaScript method after it.) I like the pure JavaScript way and also for learning purposes.

A better way is by using a template library like Handlebars.

The reason why it is better, stand on the fact you could separate logic code from your visualization code.

And more, using templates, you could have a closer look on what the page will became by looking to html code with the placeholders, that is easy to debug, instead of following complicated sequences of DOM calls.

I kindly suggest you to use such type of approach.

UPDATE

In response of your comment I'm extending my answer.

About your code, the issue I see is that you define an array of DOM element inside your function, but those elements are defined outside the scoop of your function.

This is like a nightmare to maintenance, and could easily be avoided.

Just pass the DOM object as the function arguments, and you have solved the issue:

var addEl = function(rootNode) {
    var nodes = Array.prototype.slice.call(arguments, 1);

    for(var i=0; i<nodes.length; i++) {
        rootNode.appendChild(nodes[i]);

        for(var j=0; j<3; j++){
            nodes[j].appendChild(nodes[j+1]);
        }
    }
};


Calculate the total items used, if for every x items used, you get one additional item to use I am looking for a way to optimize this code so that it runs faster. I am trying to calculate the total items used, if for every x items used, you get one  additional item to use.

This is the code I currently have. I get the correct result, but with a large amount of items, it is taking too long to calculate.

public static int TotalItems(int startingTotalItems, int newItemInterval)
{
    int itemsUsed = 0;
    int itemsUsedThisInterval = 0;
    int totalItems = startingTotalItems;

    while (totalItems > 0)
    {
        totalItems--;
        itemsUsed++;
        itemsUsedThisInterval++;
        if (itemsUsedThisInterval == newItemInterval)
        {
            totalItems++;
            itemsUsedThisInterval = 0;
        }
    }
    return itemsUsed;
}

public static void Main(string[] args)
{
    Console.WriteLine(TotalItems(3, 2));
}

Console output: 5

//-- psudo
// Start with 3 items
// Use 2 items
// Now I have 1 item plus I get 1 new additional item, giving a total of 2 items
// Use 2 items
// Now I have 0 items but since I used another 2 items, I get 1 more new additional item
// Use 1
// I have used a total of 5 items - Thisis the result
//--

I think the issue is the use of the while loop. I would think that there would be an efficient algorithm I could apply for this but but, so far, I have not had any luck finding one.

What my boss is saying is "Your code took too long to calculate an answer. Analyze how your code behaves when the input increases in size and see if you can make it work faster."

I managed to create another algorithm that produces the same results, this algorithm iteration seems to be more efficient so it will perform better.

Consider startingTotalItems as x and newItemInterval as y.

Using while loop:

public static int TotalItems(int x, int y)
{
    int sum = 0;
    while (x >= y)
    {
        int remainder = x % y;
        int quotient = x / y;
        sum += (x - remainder);
        x = remainder +  quotient;
    }
    return sum + x;
}

Using recursive method:

public static int TotalItems(int x, int y)
{
    if(x < y)
    {
        return x;
    }
    int remainder = x % y;
    int quotient = x / y;
    return x - remainder + TotalItems(remainder +  quotient, y);
}

How does it work?
Let's assume that x is the amount of items we are holding now.
For every x bigger than or equal to y we will add to the sum x without the remainder of x divided by y, let's consider that these are the items we've used during this turn.
Now we still have the remainder that we haven't used yet but we need to remember the we've got more items while using the previous items, the amount of items we've got is the quotient of x divided by y, so now x should be the quotient plus the remainder.
We will repeat this procedure until x is less than y in this case we know that we will not get any more items so all we have left to do is to add x to the sum, break the procedure and return the result.


Computing the double Integral using MonteCarlo techniques using Julia I decided to try and learn Julia for doing scientific computing, and I decided to tackle the problem of finding

$$ \int_{D_{\frac{1}{4}}} x^4 + y^2 dA $$

where \$ D_{\frac{1}{4}} \$ is the part of the unit circle in the first cuadrant.

My code in Julia is the following:

using Distributions
e = 10.0^(-3);
p = 0.85;
variance = 4;

N = floor(Int, variance / ((1-p)*((e/2)^2))) + 1

u = Uniform(0,2);
x = rand(N);
y = rand(N);
z = rand(u, N);

result = sum((x.^2 + y.^2 .<= 1) & (z .<= x.^4 + y.^2))*2.0 / N

which gives the nice  result \$ = 0.2945746303294543 \$

I kindly ask for how to improve my implementation, and reduce the footprint of memory (it uses almost 2 to 3gb in RAM).

Try swapping out the three arrays with calling a function iteratively---three Int64 arrays (each element 8 bytes) of ~100M elements is a lot. That is, switch out the body of the sum into a separate function:

function foo(u)
    x = rand()
    y = rand()
    z = rand(u)
    (x^2 + y^2 <= 1) & (z <= x^4 + y^2) ? 1 : 0
end

and replace the sum expression itself with just a loop:

count = 0
for i in 1:N
    count += foo(u)
end
count * 2.0 / N

This runs a bit faster, since the compiler can optimize the internal function, and with a much smaller memory footprint, since we sidestep the arrays entirely.


PHP Iterate over Child's Child Efficiently I have this code:

foreach($list as $id => $item_array) {
    if (($item_array['approved'] == 1) || $is_admin) {
        include('item.php');
    }
    $temp = $item_array;
    while(!empty($temp['child'])) {
        foreach($temp['child'] as $id => $item_array) {
            if (($item_array['approved'] == 1) || $is_admin) {
                include('item.php');
            }
        }
        if(empty($temp['child']['child'])) {
            unset($temp);
        }
    }
}

which functions but I'm going to need to duplicate the while block 5 times because my items can have up to six levels. There must be a more efficient way to do this, right? (The item.php just outputs HTML based on the array values)

In the simplest form $list would be:

Array
(
    [id] => 1
    [name] => Water
    [date] => 2018-05-14 13:24:22
    [content] => It's Good
    [approved] => 1
    [level] => 0
    [child] => Array([0] => Array(
                    [id] => 2
                    [name] => Hydrogen
                    [date] => 2017-05-07 15:12:14
                    [content] => Part air
                    [approved] => 1
                    [level] => 1)
                )
)

If needed I build $list with:

if($parent != -1) {
        $list[$parent]['child'][] = $this->build_list_array($id, $date, $content, $name, $approved, $level);
} else {
    $list[$id] = $this->build_list_array($id, $date, $content, $name, $approved, $level);
}
function build_comment_array($id, $date, $content, $name, $approved, $level) {
    return array('id' => $id,
                'name' => $name,
                'date' => $date,
                'content' => $content,
                'approved' => $approved,
                'level' => $level);
}

You can do it recursively. Create a function (let's say, iterateChildren(array)) that iterates over the given array and when the children key is found you call iterateChildren(item['children']).

This way you can have as many levels as you'd like, since the function calls itself at each level.

Be cautious, though, because with too many levels you may run out of memory.


Basic one way hash to store a password C# I am storing a password in a database (school coursework so nothing too complicated but I want to challenge myself), I am using a one way hash function, is this suitable? It works I just feel there may be nicer ways of storing the password. The database is SQL, language is C#

static class Encryption
{
    public static string OneWayEncrpyt(string toEncrypt)
    {
        byte[] data = System.Text.Encoding.ASCII.GetBytes("password");
        data = new System.Security.Cryptography.SHA256Managed().ComputeHash(data);
        return System.Text.Encoding.ASCII.GetString(data);
    }
}

No, it isnt.

First, you only allow ASCII characters in the password. If I were to choose €¥≠≥≤©® as my password, it would probably be equivalent to choosing ??????? (7 question marks). Try it.

Second, a password hashing function must be slow. Otherwise it is too easy to crack the password.

Third, when you hash a password, you must add some salt to it. This has the effect that even when two people use the same password, the hashed passwords are different, so a possible attacker cannot group people by their passwords.

Fourth, if you print the hashed password and it contains lots of question marks, you are doing it wrong. Instead of converting the hash bytes directly to a string, you must encode them so that only printable characters remain. Typical choices for such an encoding are base64 and hex.

In every possible scenario, even in school, you should use an established algorithm like bcrypt or scrypt instead of inventing your own.


Custom shapes objects using polymorphism in JavaScript I have made the following example and want to know if it properly represents polymorphism in JavaScript.

var customShape = function(){
    this.sides = 4;
    this.length = 5;
    this.perimeter = 4 * this.length;
};

customShape.prototype.noOfSides = function(){
    return ("Total No of Sides = " + 4);
}

var rect = new customShape();
var square = new customShape();
var triangle = new customShape();
triangle.noOfSides = function(){
    return ("Total No of Sides = " + 3);
}

console.log(rect.noOfSides());
console.log(square.noOfSides());
console.log(triangle.noOfSides());

My intention was to make a function noOfSides() which works for different objects.

References:

What is polymorphism, what is it for, and how is it used?
What is polymorphism? Explain with an example?
What is polymorphism in Javascript?
Basic javascript inheritance and polymorphism

[I] want to know if it properly represents PolyMorphism in JavaScript?

Your code could be considered a representation of polymorphism when considering that the noOfSides() function was changed for a single instance of the customShape type, but that doesn't really apply to all objects of that type. Traditionally in Object-Oriented programming, sub-typing would be used to create sub-classes that inherit the properties and methods of the base class.

You created instances of customShape, but didn't really make any objects that inherit that functionality. For instance, you made the triangle instance and added a function (like a method) to that one instance:

var triangle = new customShape();
triangle.noOfSides = function(){
    return ("Total No of Sides = " + 3);
}

Looking at the fourth reference you included, look at the Padawan inheritance example

var jedi = function(name, age) {
    Padawan.call(this, name, age);
};
jedi.prototype = Object.create(Padawan.prototype);

Similarly, you could make a constructor for a triangle. The customShape constructor doesn't appear to accept any parameters, though it could accept the number of sides, in which case the triangle constructor could pass that in as an argument to the call to customShape.call(), but for now, the sides property can be set after that call.

function triangle() {
    //initialization code
    customShape.call(this);
    this.sides = 3;
}

And then set the prototype for the triangle to an object with the customShape prototype:

triangle.prototype = Object.create(customShape.prototype);

Then you don't need to add an additional noOfSides() function for the triangle. See a demonstration below. Note that I updated the noOfSides() method to utilize this.sides instead of a hard-coded value (like Matt Fletcher mentioned in a comment on your SO question, and also did in the jsbin example he made and linked in another comment).

var customShape = function(){
    this.sides = 4;
    this.length = 5;
    this.perimeter = 4 * this.length;
};

customShape.prototype.noOfSides = function(){
    return ("Total No of Sides = " + this.sides);
}

var rect = new customShape();
var square = new customShape();
var triangle = function() {
    customShape.call(this);
    this.sides = 3;
}
triangle.prototype = Object.create(customShape.prototype);

var tri = new triangle();
console.log('rect: ', rect.noOfSides());
console.log('square: ', square.noOfSides());
console.log('triangle: ', tri.noOfSides());

Matt Fletcher has good points in his comment at 2017-11-29 12:37:29Z:

  @Deadpool Fine. This is a somewhat improved version: jsbin.com/jazasav/edit?html,js,console ... but it still makes little sense, because "length" can differ on all sides. Also these shapes and their values don't follow such simple mathematical rules. In a real world example you'd need to be able to pass in much more complex data and get it to process it in a more complex way. Perhaps use the whole "Animal" inheritance pattern to better explain it, if that's what you're planning on doing. I just would like to know what you're actually trying to gain from generating polymorphic obj

And Matt's jsbin includes an ES2015 example, which should be fsmiloar to OOP users in other languages like C++, Java, PHP, etc.

// ES6 style

class CustomShape {
  constructor(sides, length) {
    this.sides = sides;
    this.length = length;
    this.perimeter = sides * length;
  }
  noOfSides() {
    return ("Total No of Sides = " + this.sides);
  }
}

class Square extends CustomShape {
  constructor(length) {
    super(4, length);
  }
}

const mySquare = new Square(5);
console.log(mySquare.noOfSides());


Role based permissions in Express.js This is something I've done a few times, but I've found it to feel a bit error-prone with so many conditions, and am wondering if anyone can point me in the direction of a cleaner way. This is a PATCH route for editing a user. Super admin and admin users are both able to change other users (with some limitations) while other types of users can only edit themselves.
router.patch('/:userId', async (req, res) => {
  const patcher = (req as AuthRequest).user;
  const otherUser = await database.getUserById(req.params.userId);
  const requestedUpdate = req.body;

  // 404 if user is not found.
  if (!otherUser) {
    return sendCannotFind(res);
  }

  // Basic validation of requestedUpdate
  if (requestedUpdate.userType && !isUserTypeValid(requestedUpdate.userType)) {
    return sendInvalidUserType(res);
  }

  if (patcher.userType === 'superAdmin') {
    // Super admin cannot demote self.
    if (otherUser.id === patcher.id && requestedUpdate.userType) {
      return sendCannotSetUserType(res);
    }

    // Super admin cannot edit other super admins
    if (otherUser.userType === 'superAdmin' && otherUser.id !== patcher.id) {
      return sendCannotEdit(res);
    }
  } else if (patcher.userType === 'admin') {
    // Admin cannot edit super admins
    if (otherUser.userType === 'superAdmin') {
      return sendCannotEdit(res);
    }

    // Admin cannot edit other admins
    if (otherUser.userType === 'admin' && otherUser.id !== patcher.id) {
      return sendCannotEdit(res);
    }

    // Admin cannot promote or demote themselves
    if (otherUser.id === patcher.id && requestedUpdate.userType) {
      return sendCannotSetUserType(res);
    }

    // Admin cannot promote anyone to admin or superAdmin
    if (requestedUpdate.userType === 'admin' || requestedUpdate.userType === 'superAdmin') {
      return sendCannotSetUserType(res);
    }
  } else {
    // Non-admins cannot edit anyone but themselves
    if (otherUser.id !== patcher.id) {
      return sendCannotEdit(res);
    }

    // Non-admins cannot promote or demote themselves
    if (requestedUpdate.userType && requestedUpdate.userType !== otherUser.userType) {
      return sendCannotSetUserType(res);
    }
  }

  await doEdit(otherUser, requestedUpdate);
  return res.json(otherUser);
});
```

I think I came up with a better and more declarative way using Joi schemas in nested dictionaries.
The idea is that we first figure out these three things:

Is the user editing themselves? (bool)
What type of user is the editor? (admin, etc)
What type of user is being edited? (admin, etc)

And based on those three things, we can lookup the correct Joi schema to use.
So the schema dictionary looks like this for the PATCH route:
// No one is allowed to edit their own user type, everyone can edit their own name.
const selfEditSchemas: { [key in UserType]?: Joi.Schema } = {
  [UserType.superAdmin]: Joi.object({
    userType: Joi.forbidden(),
    name: Joi.string().pattern(/^[a-zA-Z0-9_\-. ]{3,100}$/).optional(),
  }),
  [UserType.admin]: Joi.object({
    userType: Joi.forbidden(),
    name: Joi.string().pattern(/^[a-zA-Z0-9_\-. ]{3,100}$/).optional(),
  }),
  [UserType.user]: Joi.object({
    userType: Joi.forbidden(),
    name: Joi.string().pattern(/^[a-zA-Z0-9_\-. ]{3,100}$/).optional(),
  }),
};

// We only allow admins and super admins to edit others, and we use different schemas
// depending on what type of user they are trying to edit.
const otherEditSchemas: { [key in UserType]: { [key in UserType]?: Joi.Schema } } = {
  [UserType.superAdmin]: {
    [UserType.admin]: Joi.object({ // (superAdmin can edit admins according to this schema)
      userType: Joi.string().valid(...Object.values(UserType)).optional(),
      name: Joi.string().pattern(/^[a-zA-Z0-9_\-. ]{3,100}$/).optional(),
    }),
    [UserType.user]: Joi.object({ // (superAdmin can edit regular users according to this chema)
      userType: Joi.string().valid(...Object.values(UserType)).optional(),
      name: Joi.string().pattern(/^[a-zA-Z0-9_\-. ]{3,100}$/).optional(),
    }),
  },
  [UserType.admin]: {
    [UserType.user]: Joi.object({ // (admin can edit regular users according to this schema)
      userType: Joi.forbidden(), // (admin cannot promote regular users)
      name: Joi.string().pattern(/^[a-zA-Z0-9_\-. ]{3,100}$/).optional(),
    }),
  },
  [UserType.user]: {}, // (user is not allowed to edit anyone else)
};

And now the logic for validating edits becomes a lot simpler and less error-prone IMO:
function validateUserEdit(req: express.Request, res: express.Response, next: express.NextFunction) {
  const userReq = req as UserRequest;

  const isSelf = userReq.user.id === userReq.otherUser.id;
  const schema = isSelf
    ? selfEditSchemas[userReq.user.userType]
    : otherEditSchemas[userReq.user.userType][userReq.otherUser.userType];

  if (!schema) {
    return res.status(403).json({
      message: 'Not allowed to edit that user.',
      code: ErrorCodes.FORBIDDEN_TO_EDIT_USER,
    });
  }

  const validateRes = schema.validate(req.body, { stripUnknown: true });
  if (validateRes.error) {
    res.status(400).json({
      message: `Invalid arguments: ${validateRes.error}`,
      code: ErrorCodes.INVALID_ARGUMENTS,
    });
  } else {
    req.body = validateRes.value;
    next();
  }
}