Reusable function for a validation process I previously asked how can I simplify a validation process:
How can I simplify this JQuery validation process?
But in hindsight, what I really meant to ask was how can I turn that process into a reusable function.
I'm going through some old code that I wrote, and I am trying to clean it up so there's not a lot of redundant code.
I have many onClick events that look like this:
$('#addSubmit').on('click', function(){
  var addcriteria = {
    addcity: $('#addcity').val(),
    addregion: $('#addregion').val(),
    addloctype: $('#addloctype').val(),
    // several more values
  }

  if(addcriteria.addcity == "" || addcriteria.addcity == null){
    $('#errorModal').modal('show');
    $('.message').text('You must enter a City.');
    $('#errorModal').on('hidden.bs.modal', function(){
        document.forms["addEquipForm"].addcriteria.addcity.focus();
    });
    return false;
  }
  if(addcriteria.addregion == ""){
    $('#errorModal').modal('show');
    $('.message').text('You must select a Region.');
    $('#errorModal').on('hidden.bs.modal', function(){
        document.forms["addEquipForm"].addcriteria.addregion.focus();
    });
    return false;
  }
  // several more checks
  else{
    $.post('api/updateEquip.php', {addcriteria:addcriteria}, function(data){
        if(data == "Success"){
            $('#successModal').modal('show');
            $('.message').text('New Equipment has been added.');
            $('#successModal').on('hidden.bs.modal', function(){
                location.reload();
            });
        }
        else{
            $('#errorModal').modal('show');
            $('.message').text('The Equipment could not be added.');
            return false;
        }
    });
  }
});

As stated, I have many onClick events that look similar to the above.  I want to get better at creating custom functions, and I think this would be a good start.
This was my attempt at creating a custom reusable function for the above code...
I started by creating the function:
function criteriaCheck(checkCriteria){
  if(checkCriteria == ""){
    $('#errorModal').modal('show');
    $('.message').text('Error message of some sort.');
    $('#errorModal').on('hidden.bs.modal', function(){
        document.forms["addEquipForm"].checkCriteria.focus();
    });
    return false;
  }
  else{
    // not sure what to return here
    // but I can console log a success message
    console.log('Success');
  }
}

Then, inside my onClick event, I can call the function and pass the value into the function:
var checkCriteria = '';

criteriaCheck(checkCriteria = addcriteria.addcity);

However, using these updates, I can only get "Success" in the console, even when I try to leave a blank form value.
With all of this said, how can I build a custom reusable function using the above onClick event?

The basic idea that I suggest is to decompose every bit of logic into small reusable functions. In my answer, I will touch only on validation logic because that part of your code has the most code duplication.
The following code is how I would like to see validation:
$('#addSubmit').on('click', function (e) {
    e.preventDefault();

    const validator = getValidator([
        [$('#addcity'), [
            combine(empty, emptyError),
            combine(notCapitalized, notCapitalizedError)]
        ],
        [$('#addregion'), [
            combine(emptyOrNull, emptyError)]
        ],
    ]);

    if (validator.hasErrors()) {
        const { element, error } = validator.getFirstError();
        $('.message').text(error.message);
        $('#errorModal').modal('show').on('hidden.bs.modal', element.focus.bind(element));
    } else {
        // Post request...
    }
});

Here we use the validator object to check if there are any errors and to get the first occurred error to display its message on the modal dialog. For the getValidator function we need to provide an element that we want to validate and a list of functions that find errors. Since functions that check for errors and actual errors are independent we need the ability to combine them. For example, the line combine(empty, emptyError) means that we want to check if a value is empty and return emptyError if so.
Also, I've made additional validation for the city field (see notCapitalizedError) to show how easy is to add new validation logic. To not hardcode error messages I've added data-name="" attributes to all inputs:
<input type="text" id="addcity" data-name="City" placeholder="City">
<input type="text" id="addregion" data-name="Region" placeholder="Region">

The implementation code is a bit hard to read and the following:
const empty = (str) => (str === "");
const emptyError = ($el) => ({ name: "empty", message: `You must enter a ${$el.data('name')}.` });
const notCapitalized = (str) => (!empty(str) && str[0] === str[0].toLowerCase());
const notCapitalizedError = ($el) => ({ name: "not-capitalized", message: `${$el.data('name')} should be capitalized.` });
const emptyOrNull = (val) => (empty(val) || val == null);

const combine = (hasError, createErr) => function($el) {
    return (hasError($el.val()) ? createErr($el) : null);
}

const getErrors = ($element, fns) => fns.map((fn) => fn($element)).filter(res => res !== null);
const getValidator = (data) => {
    const map = new Map();
    data.forEach(([$el, functions]) => {
        map.set($el, functions);
    })

    const elements = Array.from(map.keys());
    const getFunctions = ($el) => map.get($el) || [];
    const elErrors = ($el) => getErrors($el, getFunctions($el));
    const elHasErrors = ($el) => (elErrors($el).length > 0);
    const hasErrors = () => elements.some(elHasErrors);

    return {
        hasErrors,
        getFirstError: () => {
            if (hasErrors()) {
                const $element = elements.find(elHasErrors);
                return { element: $element, error: elErrors($element)[0]};
            }

            return null;
        }
    }
}


Getting rid of Merged cells Merged cells are a PITA in Excel. They prevent doing a lot of actions and make copying columns impossible. So I wrote this little code to change all the merged cells in a given sheet into 'Center across selection'.
I did NOT test against vertically merged cells, but it should not trigger an error (just unmerge them).

Sub fixMergedCells(sh As Worksheet)
'replace merged cells by Center Acroos Selection
'high perf version using a hack: https://stackoverflow.com/a/9452164/78522
    Dim c As Range, used As Range
    Dim m As Range, i As Long
    Dim constFla: constFla = Array(xlConstants, xlFormulas)

    Set used = sh.UsedRange
    For i = 0 To 1  '1 run for constants, 1 for formulas
        Err.Clear
        On Error Resume Next
        Set m = Intersect(used.Cells.SpecialCells(constFla(i)), used.Cells.SpecialCells(xlBlanks))
        On Error GoTo 0
        If Not m Is Nothing Then
            For Each c In m.Cells
                If c.MergeCells Then
                    With c.MergeArea
                        'Debug.Print .Address
                        .UnMerge
                        .HorizontalAlignment = xlCenterAcrossSelection
                    End With
                End If
            Next c
        End If
    Next i
End Sub

Sub test_fixMergedCells()
    fixMergedCells ActiveSheet
End Sub

Vertically Merged Cells

I did NOT test against vertically merged cells, but it should not trigger an error (just unmerge them).

You code will unmerge the cells and set each cells .HorizontalAlignment = xlCenterAcrossSelection.
Target Area
Changing the target area from a Worksheet to a Range would make your code more versatile.

Sub fixMergedCells(sh As Worksheet)

Sub fixMergedCells(Target As Range)
    Set Target = Intersect(Target, Target.parent.UsedRange)

SpecialCells
SpecialCells are a PITA.  I would use Range.Find("*", SearchFormat:=True).

   Application.FindFormat.MergeCells = True
   Set c = Target.Find("*", SearchFormat:=True)
   Do Until c is Nothing

Unfortunately neither SpecialCells or Range.Find are going to find Merged cells if they are blank.  It might be better to search each cell in the range.
Center Across Selection
Merged cells are not always centered.  You might want to add a condition that if centered then Center Across Selection.

 If .HorizontalAlignment = xlCenter Then .HorizontalAlignment = xlCenterAcrossSelection


Hackerrank delete a node I have been learning c++ for a few weeks now and I attempted this problem on Hackerrank Delete a node. I am aware of a similar post Delete a linked list node but the code seems more of a C structure than a C++ structure. Please feel free to suggest areas of Improvement and shorter way of writing this if any exists. Here is excerpt of the question

  You’re given the pointer to the head node of a linked list and the
  position of a node to delete. Delete the node at the given position
  and return the head node. A position of 0 indicates head, a position
  of 1 indicates one node away from the head and so on. The list may
  become empty after you delete the node.

  Input Format  You have to complete the Node* Delete(Node* head, int position) method which takes two arguments - the head of the
  linked list and the position of the node to delete. You should NOT
  read any input from stdin/console. The position will always be at least 0
  and less than the number of the elements in the list.

  Output Format  Delete the node at the given position and return the head of the updated linked list. Do NOT print anything to
  stdout/console.

  Sample Input

  1 --> 2 --> 3 --> NULL, position = 0  1 --> NULL , position = 0

  Sample Output

  2 --> 3 --> NULL NULL

/*
  Delete Node at a given position in a linked list
  Node is defined as
  struct Node
  {
     int data;
     struct Node *next;
  }
*/
Node* Delete(Node *head, int position)
{
  // Complete this method
    Node *temp = head;
    if(position == 0){
        head = head->next;
        return head;
    }
    Node *temp2 = head;
    for(int i = 0 ; i < position-1; i++){
        temp2 = temp2->next;
    }
      temp = temp2->next->next;
      temp2->next = temp;
    return head;
}

A few comments:

It is cleaner to define the position parameter as unsigned or
size_t as it cannot be negative. Same applies to the for loop counter.
There is a memory leak, as you don't do delete on the removed
node.
Names like temp and temp2 are not reader-friendly. How about
current_node? Speaking of which, the assignment to temp2 is redundant.
The code should validate that we don't go beyond the end of the list. We can of course assume that invalid index cannot be passed to the function, but it's a dangerous assumption that does not lead to defensive code.
While we're on the parameter validation topic; if we get passed an empty list (i.e. head == NULL), there will again be undefined behaviour irrespective of the value of position.
If we delete the last node, the code will behave in an undefined manner (most likely crash). Consider that temp2->next will be NULL in that case, which means that temp2->next->next won't be a happy statement.
Speaking of last comment, do you have unit tests in place? If so, it'd be good to have edge cases covered (delete first node, delete last node, empty list etc.)

Hope this helps.


Query to get distinct items at the top of results and duplicates following I need to query the table below and list the distinct items at the top of my result set and the duplicates will follow, the order of the duplicates does not matter.

I am working with Sql Server 2016 if that offers any nice tricks for improvement.

Table design

create table Items(Id INT,Item VARCHAR(5))

insert into Items values(1,'Cat')
insert into Items values(2,'Dog')
insert into Items values(3,'Dog')
insert into Items values(4,'Cat')
insert into Items values(5,'Fish')
insert into Items values(6,'Cat')
insert into Items values(7,'Dog')

Here is my current query. It does function correctly but seems very poor and brute force'ish.

--Select all the items but have the distinct ones appear at the top and repeats following
SELECT Id= (SELECT TOP 1 Id FROM Items it2 WHERE it2.Item = it1.Item ORDER BY Id ASC), Item
FROM Items it1
GROUP BY Item

UNION ALL

SELECT *
FROM Items WHERE Items.Id NOT IN
(
    SELECT Id= (SELECT TOP 1 Id FROM Items it2 WHERE it2.Item = it1.Item ORDER BY Id ASC)
    FROM Items it1
    GROUP BY Item
)

Results

1, Cat
2, Dog
5, Fish
3, Dog
4, Cat
6, Cat
7, Dog

That's a simple task for Windowed Aggregates :-)

Assign a sequential number (1,2,3,...) to each row with the same Item and order by that number:

WITH cte AS
 (
   SELECT Id, Item,
      ROW_NUMBER()
      Over (PARTITION BY Item
            ORDER BY Item) AS rn
   FROM Items
 )
SELECT Id, Item
FROM cte
ORDER BY rn, Item

Another way replaces ROW_NUMBER with

      COUNT(*)
      Over (PARTITION BY Item
            ROWS UNBOUNDED PRECEDING) AS rn

This should be more efficient as it doesn't need to actually sort.
I can't test it right now, but it should be similar to a trick used in older versions of SQL Server (before Cumulative aggregates have been implemented):

      ROW_NUMBER()
      Over (PARTITION BY Item
            ORDER BY (SELECT 1)) AS rn


Splitting an array in 2 parts such that one part sums to a multiple of 10 and other sums to an odd number Given an array of ints, is it possible to divide the ints into two
  groups, so that the sum of one group is a multiple of 10, and the sum
  of the other group is odd. Every int must be in one group or the
  other. Write a recursive helper method that takes whatever arguments
  you like, and make the initial call to your recursive helper from
  splitOdd10(). (No loops needed.)

splitOdd10({5, 5, 5}) → true

splitOdd10({5, 5, 6}) → false

splitOdd10({5, 5, 6, 1}) → true

public boolean splitOdd10(int[] nums) {
        int total=findSum(0, nums, 0);
        if((total%10)%2==0||total==0)
        {
             return false;
        }
        return ifGroupExistsThatSumsToValue(0, nums, total%10);
    }
    public int findSum(int n, int[] nums, int sum)
    {
        if(nums.length==0)
            return 0;
        if(n==nums.length-1)
        {
            return sum+nums[n];
        }
        return findSum(n+1, nums, sum+nums[n]);
    }

    public boolean ifGroupExistsThatSumsToValue(int start, int[]nums, int target)
    {
       if(start==nums.length-1)
       {
           return target-nums[start]==0;
       }
       if(target==0)
       {
           return true;
       }
       if(ifGroupExistsThatSumsToValue(start+1, nums, target-nums[start]))
          return ifGroupExistsThatSumsToValue(start+1, nums, target-nums[start]);

       return ifGroupExistsThatSumsToValue(start+1, nums, target+nums[start]);
       }

You could simplify findSum. I would also rename the variables and reorder the parameters:

private int findSum(int[] nums, int i, int accum) {
    if (i == nums.length) {
        return accum;
    }
    return findSum(nums, i + 1, nums[i] + accum);
}

The end part of the ifGroupExistsThatSumsToValue method can also be simplified:

public boolean ifGroupExistsThatSumsToValue(int start, int[] nums, int target) {
    if (start == nums.length - 1) {
        return target - nums[start] == 0;
    }
    if (target == 0) {
        return true;
    }
    return ifGroupExistsThatSumsToValue(start + 1, nums, target - nums[start])
            || ifGroupExistsThatSumsToValue(start + 1, nums, target + nums[start]);
}

When logic is kind of tricky, like in this problem, it's good to add the examples in the problem statement as proper unit tests:

@Test
public void test_5_5_5() {
    assertTrue(splitOdd10(new int[]{5, 5, 5}));
}

@Test
public void test_5_5_6() {
    assertFalse(splitOdd10(new int[]{5, 5, 6}));
}

@Test
public void test_5_5_6_1() {
    assertTrue(splitOdd10(new int[]{5, 5, 6, 1}));
}

And then keep adding some more to try to cover all corner cases, for example:

@Test
public void testEmpty() {
    assertFalse(splitOdd10(new int[0]));
}

@Test
public void testSingleNum() {
    assertTrue(splitOdd10(new int[]{1}));
    assertFalse(splitOdd10(new int[]{2}));
    assertFalse(splitOdd10(new int[]{10}));
}

As others have pointed out, your formatting is awful. Please reformat nicely next time, Control Shift f in Eclipse does this easily, and any decent IDE has the equivalent.

To be honest, I don't really understand why splitOdd10 and ifGroupExistsThatSumsToValue have to be so complicated. If I simply follow the problem description in a straightforward way and just try to add up the numbers from left to right and check the conditions, I arrive at something that's much shorter and easier to understand:

private boolean splitOdd10(int[] nums) {
    return splitOdd10(nums, 0, 0, findSum(nums, 0, 0));
}

private boolean splitOdd10(int[] nums, int i, int left, int right) {
    return left % 10 == 0 && right % 2 == 1
            || i < nums.length && splitOdd10(nums, i + 1, left + nums[i], right - nums[i]);
}

I'm wondering if I'm missing something. Both your and my solutions are passing all my unit tests.


Improving Tooltips with json data This is a working example that I am trying to improve.

Throughout the application I am working on one of the requirements was to have tooltips everywhere.  However the customer wanted the ability to see a "Summary" of these tooltips and be able to manage them from a single location.

What I ended up doing is creating a new class called FormatToolTip.js

function FormatToolTip() {
    var self = this;
    amplify.request.define("toolTipListing", "ajax", {
        url: "resources/prototype/tooltips.json",
        dataType: "json",
        type: "GET"
    });
    self.ToolTipId = "ToolTipId";
    self.allToolTips = "ToolTips";
    self.init = function () {
        amplify.request({
            resourceId: "toolTipListing",
            success: function (data) {
                amplify.store(self.allToolTips, data.toolTips);
            },
            error: function () {
            }
        });
    };

self.buildToolTip = function (helpId) {
    var toolTipList = amplify.store(self.allToolTips);
    for (var i = 0; i < toolTipList.length; i++) {
        var val = toolTipList[i];
        if (helpId == val.id) {
            text = val.text;
            return text;
        }
    }
    return amplify.store(self.ToolTipId);
};
self.setToolTipId = function (toolTipId) {
    if (toolTipId != undefined || toolTipId != null) {
        amplify.store(self.ToolTipId, toolTipId);
    }
};
self.init();
}

I wanted everything to be based off a class I assign to the element.   Here is an example of the format I used.

From there it can be initialized.

To polish off the look and feel I added some CSS and a background image for the element.

.withToolTip{
    background-image:url("images/help.png");
    background-repeat: no-repeat;
    background-position: center;
    background-size: 20px;
    width: 20px;
    height: 20px;
    display: inline-block;
    vertical-align: middle;
    padding-left: 10px;
}

I am looking to improve on my code. So suggestions would be awesome.

function FormatToolTip() {
    "use strict";

    amplify.request.define("listTooltips", "ajax", {
        url: "resources/prototype/tooltips.json",
        dataType: "json",
        type: "GET"
    });

    amplify.request({
        resourceId: "listTooltips",
        success: function (data) {
            amplify.store('tooltips', data.toolTips);
        }
    });

    // Get the tooltip from storage, given it's id
    this.buildToolTip = function (id) {
        var tips = amplify.store('tooltips');
        for (var i = 0, len = tips.length; i < len; i++) {
            var val = tips[i];
            if (id == val.id) {
                return val.text;
            }
        }

        // This doesn't appear to be used?
        //return amplify.store(this.ToolTipId);
    };

    // Don't think this is used either
    /*
    this.setToolTipId = function (toolTipId) {
        if (toolTipId != undefined || toolTipId != null) {
            amplify.store(this.ToolTipId, toolTipId);
        }
    };
    */
}

Just use a data attribute in your markup:

And initialize like so:

* Untested


What improvements can I make to avoid these If statments? I have Model Object Car.

public class Car
{
  private String colour;
  private String make;
  private String yearOfMake;
}

Then a managed bean

public Class CarSearchManagedBean
{
  private Car car;

  @EJB
  private CarFacade carFacade;

  public String search()
  {
     this.car =carFacade.find(car);
     .....
  }
}

Then I have a JSF facelet with input fields connected to car object by backing bean and a search button linked to search method .There is no Ajax and use can provide values for any property and since there is no auto cleaning can possibly provide values more than one property but search only uses one.

Then before I show CarFacade I will show CarDao becasue I think makes more sense here.

public class CarDaoImpl implements CarDao
{
  public Car findByColour(Car car){
  //Calls a webservice method
  }
  public Car findByMake(Car car){
  //Calls a webservice method
  }
  public Car findByYearOfMake(Car car){
  //Calls a webservice method
  }

}

Now if the CarFacade I have this ugly if statment that I do not know how to avoid.

public class CarFacade

{
   @EJB
   CarDao carDao;

   public Car find(final Car car)
   {
      Car aCar ;
      //How can improve the design to avoid this If statment
      if (car.getColour()!=null)
      {
       aCar = carDao.findByColour(car)
      }
      else if (car.getMake()!=null)
      {
        aCar = carDao.findByMake(car);
      }
      else
      {
        aCar =carDao.findByYearOfMake(car);
      }
      return aCar;
   }
}

You'll want to look up the Strategy Pattern.

In your specific case, you're probably going to want to do something along these lines (note - I've never worked with ManagedBeans, so I don't know what spanners that throws).

Leave your Car class as normal.  Or, well, you may want to provide specific types for assignments, if possible (So make Colour an actual class, even if it only contains an actual String - just so people can't assign a Make value to it...).

Your CarDAO should be multiple instances.  Something like:

public class CarColorFinderDAOImpl implements CarDAO {

    public Car find(String colorToFind) {
        // search by color here
    }
}

Then your CarFacade actually transforms to something like this:

public class CarFacade {

    private Map<String, CarDAO> finderDAO = new HashMap<String, CarDAO>();

    public Car find(Sring searchValue, String searchColumn) {

       return finderDAO.get(searchColumn).find(searchValue);
    }

    public void registerFinder(String searchColumn, CarDAO finder) {
        finderDAO.put(searchColumn, finder);
    }
}

Which you then call (mostly) as normal from your managed bean.
Please not that there are further abstractions available (such as encapsulating searchValue and searchColumn in some sort of FinderContract object).  But this is a quick basic one to get you started.


SQL Query that pulls rows in current table based on parent id optimization I am creating a user permission system. I will have my user permissions set up off of "objects" and not pages. For example, I have an EVENTS section. Within that section, there are FORMS, FEES, etc. I want to be able to handle permissions down to the child levels.

Here is my permission_objects table:

| id | object | parent_id |
+----+--------+-----------+
| 1  | EVENTS | NULL      |
| 2  | FORMS  | 1         |
| 3  | FEES   | 1         |

Here is my current query:

SELECT `up`.*
FROM `permission_objects` `po`
INNER JOIN `user_permissions` `up` ON `up`.`object`=`po`.`id` AND (`up`.`user_group`=2 OR `up`.`user_id`=17)
WHERE `po`.`object`='EVENTS' OR `po`.`parent_id`=(
  SELECT `id` FROM `permission_objects` WHERE `object`='EVENTS'
)

I really want to avoid doing a subquery. I thought that instead of having a parent_id I could have parent_object and have that contain the parent's object value. i.e. | 2 | FORMS | EVENTS |. That way I could change my WHERE clause to:

WHERE `po`.`object`='EVENTS' OR `po`.`parent_object`='EVENTS'

However I feel that keeping it as parent_id is a better approach. Is there a way to keep that but not have to have a subquery in my sql?

The sub-query as a concept is not a problem... you need to do that work, and this is a decent way to do it.

It would be better if it was expressed as a direct lookup... like:

WHERE `po`.`object`='EVENTS'
   OR exists (
      SELECT `sub`.`id`
      FROM `permission_objects` `sub`
      WHERE `sub`.`object`='EVENTS'
        and `sub`.`id` = `po`.`parent_id`
)

This sub-select is better because it can do an indexed lookup in the table in the sub-select based on the id column.

Apart from that, I don't see a better, or neater way to do it.


Geometry table for use in regexes I have been teaching myself Python 3. I watched a video on YouTube entitled "stop writing classes". The Gtable class doesn't even have an init function, so I can't help but wonder if I should have done something different. Also as someone teaching themselves a language for the first time I would really appreciate any critique on how to write the code more efficiently, as well as any anti-idioms unintentionally used. Specifically I feel like there was probably a better way to generate the dictionaries, but especially with the first one I was having trouble writing a comprehension.

##input file text
##|HAAS T|TARGET T|XGEOM|ZGEOM
##$T0101$T0101$X22.960$Z20.0$
##$T0202$T0606$X32.425$Z7.376$
##$T0303$T0404$X30.588$Z15.620$
##$T0404$T0202$X22.367$Z18.549$
##$T0505$T0808$X33.291$Z8.197$

class GeometryTable(dict):
    "create a geometry table from a file for use in regexes"
    def __init__(self):
        self.d={}
    def create(self,location):
        f=open(location,'r')
        tablestring=f.read()
        f.close()
        tablelist=[]
        tablelist=tablestring.split('$')
        #delete unecessary data from list
        tablelist.pop(0)
        tablelist=[item for item in tablelist if item!='\n']
        #create a dictionary
        count=0
        keylist=[]
        valuelist=[]
        while count<len(tablelist):
            keylist.append(tablelist[count])
            valuelist.append([tablelist[count+1],tablelist[count+2],tablelist[count+3]])
            count+=4
        self.d=dict(zip(keylist,valuelist))
    def xval(self,string):
        if string in self.d:
            return (self.d[string][1])
        else:return(r"X20.0")
    def zval(self,string):
        if string in self.d:
            return (self.d[string][2])
        else:return(r"Z20.0")
    def tval(self,string):
        if string in self.d:
            return (self.d[string][0])
        else:return(r"T2020")
    def report(self,string):
        if string in self.d:
            return (self.d[string])
        else:return(r"(no value)")

class GTable(GeometryTable):
    """for replacing geometries that were given
       default values before this program was created"""
    def load(self,location):
        GeometryTable.load(self,location)
        b=self.__dict__["d"].values()
        redict={x[0]:x for x in b}
        self.d=redict

##output dictionaries for regular expressions
##>>> test.d
##{'T0101': ['T0101', 'X22.960', 'Z20.0'],
## 'T0202': ['T0606', 'X32.425', 'Z7.376'],
## 'T0303': ['T0404', 'X30.588', 'Z15.620'],
## 'T0404': ['T0202', 'X22.367', 'Z18.549'],
## 'T0505': ['T0808', 'X33.291', 'Z8.197']}

So there are a few improvements that I can see in your code, mainly in the create function.

When reading a file, use the with keyword. This will open the file and, once the program leaves that scope, the file will automatically be closed.

I would recommend changing how you read the file. From what I can tell, each line will be its own key in your dict. So instead of reading the entire file in one statement, I would recommend reading line-by-line.

By making the changes mentioned above, when we split the input by $, we get unnecessary values in the first and last index of the resulting list. To fix this, we can use list slicing. By using the notation list[start:end], where start specifies the first index you want included and stop specifies the first index you want excluded, we can remove the first and last indices.

Finally, when creating a dict you can assign a key-value pair simply by using the syntax:

dict[key] = value

Bringing all of these suggestions together, here is my version of your create function:

def create(self, location):
    # Once this block of code finishes, f will close.
    with open(location, 'r') as f:
        # Reads the file line-by-line
        for line in f:
            # Split the data and remove the first and last indices
            data = line.split('$')[1:-1]

            # Assign the key-value pair to the dict
            self.d[data[0]] = data[1:]

As a final aside, look over PEP8. PEP8 is the official Python style guide. Following those conventions (especially underscores in variable names, so table_string instead of tablestring) will help your code look and feel more Pythonic.


Web Scraping, Serializing and Deserializing in Python I scraped data from a local .html page and the below code is working. I am a newbie to scraping, just tried with a simple HTML page. It takes 10 sec to complete execution and print data. If I did anything wrong or need improvements, please let me know.
from bs4 import BeautifulSoup
import json
import pyodbc
import datetime

class JsonClass:
    def __init__(self, Date, DocumentType, Procedure, NoOfPages):
        self.Date = Date
        self.DocumentType = DocumentType
        self.Procedure = Procedure
        self.NoOfPages = NoOfPages

def json_to_db(json_string):
    conn = pyodbc.connect('Driver={SQL Server};' 'Server=xyz-PAVILION;' 'Database=jsondata;' 'Trusted_Connect=yes')
    conn.autocommit = True
    cursor = conn.cursor()
    try:
        cursor.execute('EXEC prcJsonInsertData @json = ?',
                   json_string)  # Passing Json Data to DB via Stored Procedure(SP)
        print('Data inserted')
    except pyodbc.Error as error:
        print('Error : %s' % error)
        return False
    except:
        print('Operation Failed')
        return False
    conn.close()
    return True

def json_serialize(dict_list):
    with open('html_to_json.json', 'w') as file_out:
        json.dump(dict_list, file_out, indent=4)  # Serializing dict_list and writing in  .json file
    return json.dumps(dict_list)

def json_deserilaize(json_string):
    with open('html_to_json.json', 'r') as file_out:
    json_data = json.load(file_out)  # Deserialization Data
    json_class = [JsonClass(**i) for i in json_data]  # Binding Json_data to Json_Class
    print('********* After Deserialization *******************')
    print('-------------------------------------')
    for i in json_class:
        print('Date : ' + i.Date)
        print('DocumentType : ' + i.DocumentType)
        print('Procedure : ' + i.Procedure)
        print('NoOfPages : ' + i.NoOfPages)
        print('-------------------------------------')

def html_data():
    my_file = open("C:/Users/xyz/Downloads/sample.htm", 'r')
    soup = BeautifulSoup(my_file, 'html.parser', from_encoding="UTF-8")
    t_body = soup.find('tbody')
    rows = t_body.find_all('tr')
    dict_list = []
    for row in rows:
        column = row.find_all('td')
        column = [x.text for x in column]
        record = dict()
        date_obj = datetime.datetime.strptime(column[1], '%d.%m.%Y')
        record['Date'] = date_obj.date().isoformat()
        record['DocumentType'] = column[2]
        record['Procedure'] = column[3].replace('\u00a0/\u00a0', '/').replace('\u00a0', '/')
        record['NoOfPages'] = column[4]
        dict_list.append(record)
    json_string = json_serialize(dict_list)  # Func - 1
    if json_to_db(json_string):  # Func - 2
        json_deserilaize(json_string)  # Func - 3

if __name__ == '__main__':
    html_data()

PEP8 names
All of these variables:
def __init__(self, Date, DocumentType, Procedure, NoOfPages):
    self.Date = Date
    self.DocumentType = DocumentType
    self.Procedure = Procedure
    self.NoOfPages = NoOfPages

should be lower_snake_case.
Data classes
Replace JsonClass with a @dataclass that uses an implicit __init__.
Connection
First note that there's no point in separating out your connection parameters like
'Driver={SQL Server};' 'Server=xyz-PAVILION;'

since they will be implicitly concatenated to
'Driver={SQL Server};Server=xyz-PAVILION;'

Beyond that, the documentation states that kwargs are converted to a formatted conn string, so you're better off writing
pyodbc.connect(
    Driver='{SQL Server}',
    Server='xyz-PAVILION',
    Database='jsondata',
    Trusted_Connect='yes',
)

Context management
Use a with on your connection and cursor objects; read
https://github.com/mkleehammer/pyodbc/wiki/Connection#context-manager
Exception interference
Don't convert exceptions to booleans, as in
except pyodbc.Error as error:
    print('Error : %s' % error)
    return False
except:
    print('Operation Failed')
    return False
conn.close()
return True

Deal with the exceptions, potentially wrapping them in your own exception types, and catching them at an upper level.
Double serialization
Don't do this:
with open('html_to_json.json', 'w') as file_out:
    json.dump(dict_list, file_out, indent=4)  # Serializing dict_list and writing in  .json file
return json.dumps(dict_list)

Hold onto the result of dumps and write that string to the file.
Print helpers
Move this code:
    print('Date : ' + i.Date)
    print('DocumentType : ' + i.DocumentType)
    print('Procedure : ' + i.Procedure)
    print('NoOfPages : ' + i.NoOfPages)

to a method of JsonClass.
Hard-coded paths
my_file = open("C:/Users/xyz/Downloads/sample.htm", 'r')

should not be hard-coded. Set it as some kind of parameter - a command-line argument maybe.
Dict literals
    record = dict()
    date_obj = datetime.datetime.strptime(column[1], '%d.%m.%Y')
    record['Date'] = date_obj.date().isoformat()
    record['DocumentType'] = column[2]
    record['Procedure'] = column[3].replace('\u00a0/\u00a0', '/').replace('\u00a0', '/')
    record['NoOfPages'] = column[4]

should not call dict(), should not individually index keys, and should instead use a dict {} literal.


Scrolling with an easing function I wanted to write a function that when invoked, will scroll the page downwards in a smooth fashion.

Comments and criticism welcome. I don't like, for example that determining whether it is done is not based on the scroll position.

const EASING = BezierEasing(0, .1, .1, 1); // Example values.
const DURATION = 1000; // ms.

document.addEventListener('DOMContentLoaded',
  () =>  {
    document.querySelector('#foo')
      .addEventListener('click', onClick, false);
});

function onClick(e) {
  scroll(400); // px.
  e.preventDefault();
  return false;
}

function scroll(distance) {
  var start, travelled, html;

  start = performance.now();
  travelled = 0;
  html = document.querySelector('html');

  (function move(now) {
    var fractionDone, dy, toMove;

    fractionDone = (now-start) / DURATION;

    if((1 - fractionDone) <= 0) {
      return; // Done!
    }

    if(window.scrollY + window.innerHeight
      === html.offsetHeight) {
      return; // At bottom of page.
    }

    dy = ((EASING.get(fractionDone)) * distance);
    toMove = Math.floor((dy - travelled)); // `scrollBy` only accepts integers.

    if(toMove > 0) {
      window.scrollBy(0, toMove);
      travelled += toMove;
    }

    requestAnimationFrame(move);
  }(start));
}
<!DOCTYPE html>
<html>
  <head>
    <script src="https://rawgit.com/gre/bezier-easing/master/build.js"></script>
  </head>

  <body>
    <a href id="foo">Click Me!</a>
    <script>
      var body = document.querySelector('body');
      for(var x = 0; x < 50; x++) {
        var div = document.createElement("div");
        var text = document.createTextNode(x);
        div.appendChild(text);
        body.appendChild(div);
      }
    </script>
  </body>
</html>

Now assuming you simply just want to use standard APIs and ES6 (because from a cross-browser point of view, this code isn't guaranteed to work)...

onClick is unnecessarily pulled out. You can just inline it. Also, I believe return false are unnecessary when you already have preventDefault. You wouldn't want to prevent bubbling as something else might be delegating their handlers.

document.addEventListener('DOMContentLoaded', () =>  {
    document.querySelector('#foo').addEventListener('click', e => {
      event.preventDefault();
      scroll(400);
    }, false);
});

Variable declarations are also unnecessarily separated. It's not like they're defined somewhere in the middle (where you sometimes need to pull them up for visibility).

var start = performance.now();
var travelled = 0;
var html = document.querySelector('html');

Pull out your move function from your scroll. What happens is that every call to scroll, you're creating move.
I also suggest you schedule you call requestAnimationFrame first thing in the callback. That's so your code won't hold up the scheduling of the next frame. Should you want to bail out, you can always use cancelAnimationFrame.
Math.floor can be substituted by | 0 (bitwise or). Under the hood, it transforms the number from float to int, dropping the decimals. It's fast one some engines, but your mileage may vary.
Your "bail out" conditions can simply be combined:

if((1 - fractionDone) <= 0 || window.scrollY + window.innerHeight === html.offsetHeight) return


C++11 Quicksort any container As the title says, simple quicksort to help myself get used to C++ templates and iterators.  Main concern is whether there's a better way to template so that you can have a simple function (e.g. quicksort) that accepts any container type (specifically should I have some kind of assert or more complicated template to make sure the arguments are in fact iterators?)  Also note that it is inclusive of the end value, so to sort a whole container the call is qsort(c.begin(), c.end()-1).

template <typename Iter> Iter partition(Iter start, Iter end){
    Iter pivot = start;
    Iter cur = start;
    while (cur < end) {
        if(*cur <= *end){
            std::swap(*cur,*pivot);
            pivot++;
        }
        cur++;
    }
    std::swap(*pivot, *end);
    return pivot;
}

template <typename Iter> void qsort(Iter start, Iter end){
    if(end > start) {
        Iter p = partition(start, end);
        qsort(start, p - 1);
        qsort(p + 1, end);
    }
}

If you make restriction only for ForwardIterators you can't call c.end()-1 from your question. It would be probably difficult to sort the whole container.


Parallel processing in different directories in Perl I have some subroutines which I commonly copy & paste into much of my work.
I recently posted this on StackOverflow
and user simbabque suggested that I post my code here for reviews and comments:

#!/usr/bin/env perl

use strict; use warnings; use Cwd;
my $TOP_DIRECTORY = getcwd();
local $SIG{__WARN__} = sub {#kill the program if there are any warnings
    my $message = shift;
    my $fail_filename = "$TOP_DIRECTORY/$0.FAIL";
    open my $fh, '>', $fail_filename or die "Can't write $fail_filename: $!";
    printf $fh ("$message @ %s\n", getcwd());
    close $fh;
    die "$message\n";
};#http://perlmaven.com/how-to-capture-and-save-warnings-in-perl

sub execute {
    my $command = shift;
    print "Executing Command: $command\n";
    if (system($command) != 0) {
        my $fail_filename = "$TOP_DIRECTORY/$0.fail";
        open my $fh, '>', $fail_filename or die "Can't write $fail_filename: $!";
        print $fh "$command failed.\n";
        close $fh;
        print "$command failed.\n";
        die;
    }
}

sub run_parallel {
    my $command_array_reference = shift;
    unless ((ref $command_array_reference) =~ m/ARRAY/) {
        print "run_parallel requires an array reference as input.\n";
        die;
    }
    use Parallel::ForkManager;
    my $manager = new Parallel::ForkManager(4);
    my $START_DIRECTORY = getcwd();
    foreach my $command (0..scalar @{ $command_array_reference }-1) {
        $manager->start and next;
        my ($dir, $cmd) = @{ $command_array_reference->[$command] };
        chdir $dir or die "Can't chdir to $dir: $!";
        execute( $cmd );
        chdir $START_DIRECTORY or die "Can't chdir to $START_DIRECTORY: $!";
        $manager->finish;
    }
    $manager->wait_all_children;#necessary after all lists
}

my @commands;

$commands[0][0] = 'tmp1';#directory
$commands[0][1] = 'echo "ave Maria" > aveMaria.txt';#command

$commands[1][0] = 'tmp2';#directory
$commands[1][1] = 'echo "IN HOC SIGNO VINCES" > xp.txt';#command

run_parallel(\@commands);

I'll do a first pass where I point out syntax and Perl conventions, mostly line by line. I will then talk about alternative strategies to make it more efficient and maintainable.

Syntax

use strict; use warnings; use Cwd;

You have use strict and use warnings. Well done!

You would typically have one use or require statement per line. That way, it's easier to read them.

You import getcwd from Cwd. It's a good practice to never have default imports, but to either specifically import the stuff you want, or not to import any names at all. This way, you don't pollute your namespace (even though you are in main implicitly), and later you know where functions in your code came from.

use strict;
use warnings;
use Cwd 'getcwd';

open my $fh, '>', $fail_filename or die "Can't write $fail_filename: $!";

You are using three-argument open and lexical filehandles. That's great. Those tutorials are paying off.

die "$message\n";

If you include a trailing newline "\n" in your message to die, it will not tell you what line it died on. You are mixing both versions. I think you should stick with the line numbers. Get rid of those newline, and remember that it's important to be consistent in programming.

sub run_parallel {
    my $command_array_reference = shift;

I guess you read somewhere that variable names should be speaking. Or maybe you don't want to get confused about the kind of data this variable holds. Speaking names are great. But you don't really need to name your variable foo_array_reference. The fact that its sigil is a $ already tells you it's a scalar, and thus likely a reference. A better name would be $commands, where the plural indicates it's a list of some kind. If you want to write down that it's an array, write a comment or some POD over the function.

=head2 run_parallel

Takes an array reference of commands, spawns four workers to work through
all the commands in parallel and returns the return
value of C<$manager->wait_all_children>.

=cut

sub run_parallel {
    my $commands = shift;

As you can see from my documentation, we have another issue coming up.

unless ((ref $command_array_reference) =~ m/ARRAY/) {
    print "run_parallel requires an array reference as input.\n";
    die;
}

There is a bit too much stuff here. You don't need a regular expression match to check the ref. It will return "ARRAY", so you could say:

ref $commands eq 'ARRAY'

However, that will give a use of unitialized value warning if we pass a string or a number, or something else that is not a reference. If you really want to check this, and don't want to write

unless ( ref $commands and ref $commands eq 'ARRAY') { ... }

you should take a look at Ref::Util, which provides a lot of useful functions to check this kind of thing in a very fast way, as it's written in XS. In your case, you could use is_arrayref.

If you insist on dieing, then do it properly. Don't print to STDOUT and then die without a message, so that it just says Died at line ... to STDERR. die can take an error message, but shouldn't have the newline \n, or it will not report the line number.

You can then use a nice post-fix unless.

use strict;
# ...
use Ref::Util 'is_arrayref';

sub run_parallel {
    # ...

    die 'run_parallel requires an array reference as input'
        unless is_array($commands);

use Parallel::ForkManager;
my $manager = new Parallel::ForkManager(4);

When Perl executes your program, there are two phases. It first reads the whole thing and compiles it. This phase is called compile time. Then it actually runs the program, which is called run time. use statements are resolved during compile time. That means it doesn't matter if they are inside a sub, or an if. They will always be found and executed. Conditional use statements are a code smell. Put the use at the top with the others.

The old-fashioned new Module(@args) form (called indirect object notation) has been out of date for a long time. Instead, you should use the more modern version, which will take care of inheritance properly.

my $manager = Parallel::ForkManager(4);

my $START_DIRECTORY = getcwd();

ALL_CAPS variable names usually indicate constants in Perl. Your variable is lexical. Name it $start_directory instead.

You don't have to do the parenthesis after a function call if Perl knows it's a function (it was imported from Cwd, so it knows that). It's ok to do them, but it's important to be consistent if you do, and have them all the time. You do that, so this is fine.

foreach my $command (0..scalar @{ $command_array_reference }-1) {
    # ...
    my ($dir, $cmd) = @{ $command_array_reference->[$command] };
    # ...
}

You are making your life harder than you have to here. Using a range and foreach is bettern than a C-style for loop, but it's not the easiest choice in this situation.

I would iterate the array reference directly. Instead of calling it in scalar context, just walk over its elements. The dereferencing operator @{} returns a list of all of them if you put it directly into the parens. This will make it easier to grab the two elements and might have saved you the question over on Stack Overflow.

foreach my $command (@{ $commands }) {
    # ...
    my ($dir, $cmd) = @{ $command };
    # ...
}

By the way, if you have a Perl that's at least version 5.24 you can use the post-fix dereferencing operator. It's easier to read because you can go left to right all the way, but if you're used to the old stuff (like me) it's hard to get used to it. Also, my commercial Perl IDE doesn't highlight it yet.

foreach my $command ( $commands->@* ) {
    my ($dir, $cmd) = $command->@*;
}

my @commands;

$commands[0][0] = 'tmp1';#directory
$commands[0][1] = 'echo "ave Maria" > aveMaria.txt';#command

$commands[1][0] = 'tmp2';#directory
$commands[1][1] = 'echo "IN HOC SIGNO VINCES" > xp.txt';#command

You can write these kinds of structures the same way Data::Dumper outputs them. That makes it easier to read.

my @commands = (
    [
        'tmp1', # directory
        'echo "ave Maria" > aveMaria.txt', # command
    ],
    [
        'tmp2', # directory
        'echo "IN HOC SIGNO VINCES" > xp.txt', # command
    ],
);

Also note that there is a convention to have space in front and after the # for comments, as that makes it easier to read.

In general, I prefer to use references directly when I know I'll take a reference of the variable. Especially for configuration. And since you never actually do anything else with @commands, there is no need to create it in the first place.

run_parallel(
    [
        [
            'tmp1',                               # directory
            'echo "ave Maria" > aveMaria.txt',    #command
        ],
        [
            'tmp2',                                   # directory
            'echo "IN HOC SIGNO VINCES" > xp.txt',    #command
        ],
    ]
);

Maintainability

You have some code duplication in the signal handler and execute. Opening and writing the file is implemented twice, with only the same error message being different. It would be nicer to write a function that does all of that, and pass it the message. You can then use that function in the signal handler as well as execute.

There is also no need to explicitly close, as $fh is lexical and will be cleaned up automatically when it goes out of scope, or in this case, the program ends.

sub log_error_and_die {
    my $error = shift;

    my $fail_filename = "$TOP_DIRECTORY/$0.FAIL";
    open my $fh, '>', $fail_filename or die "Can't write $fail_filename: $!";
    print $fh $error;

    die $error;
}

And you can call it like this

# kill the program if there are any warnings
local $SIG{__WARN__} = sub {
    my $message = shift;
    log_error_and_die( sprintf( '%s @ %s', $message, getcwd() ) );
};

and

sub execute {
    my $command = shift;
    print "Executing Command: $command\n";
    if (system($command) != 0) {
         log_error_and_die("$command failed.");
    }
}

Why are there always four jobs? If there are a lot of files, wouldn't it be easier to make this an option? Especially with your limited input data, four jobs are overkill. But they might be not enough.

I would add a second argument to run_parallel and pass the number of parallel jobs.

sub run_parallel {
    my ($commands, $jobs) = @_; # alternate way to get arguments
    # ...

    my $manager = Parallel::ForkManager($jobs);
}

Of course, since there is nothing else, you can also get rid of that function all together. If you only ever call it once from the body of your script, I believe there's no need to make it into a function. Opinions vary on this, though.

Other considerations

    chdir $dir or die "Can't chdir to $dir: $!";
    execute( $cmd );
    chdir $START_DIRECTORY or die "Can't chdir to $START_DIRECTORY: $!";

Why do you chdir before calling execute? Is that not part of the execution? I think I would move that into execute.

Furthermore, why do you chdir back afterwards? That's already the working directory of the child, which will be reaped a few lines down. At that point, its working directory doesn't matter, and it will never have any influence on the parent's or the sibblings' working directory. You can safely remove that second chdir.

Once you've done that, you can pass $command along to execute and have a cleaner interface.

$commands[0][0] = 'tmp1';#directory
$commands[0][1] = 'echo "ave Maria" > aveMaria.txt';#command

The inner data structure of your commands could be a hash reference as well. That would make it a bit more work to write it all down, but you would not need those comments any more, and it will be way easier to read half a year later.

my $commands = [
    {
        dir => 'tmp1',
        cmd => 'echo "ave Maria" > aveMaria.txt',
    },
    # ...
];

Then to work with it in your loop, you don't have to assign $dir and $cmd any more.

sub execute {
    my $command = shift;
    chdir $command->{dir}
        or die "Can't chdir to $command->{dir}: $!";

    print "Executing Command: $command->{cmd}\n";
    if (system($command->{cmd}) != 0) {
         log_error_and_die("$command->{cmd} failed.");
    }
}

# ...

foreach my $command (@{ $commands }) {
    $manager->start and next;
    execute($command);
    $manager->finish;
}

Instead of all the die calls you can use the autodie pragma, which will turn all failures into die. It can make the code a lot more readable.

use autodie 'all'; # includes system()

Instead of the signal handler, you can set all warnings to be FATAL on the warnigns pragma.

use warnings FATAL => 'all'; # every warning dies


Tic-Tac-Toe command line I created a simple Tic-Tac-Toe.  I'm looking for any critiques from a code standpoint: general structure / style / efficiency.

Unfortunately I couldn't get a live demo working because of the dependency on the NodeJS library prompt.

/*  A simple Tic-Tac-Toe game implemented in Javascript V8 3.14.5.9

Players 'X' and 'O' take turn inputing their position on the command line using numbers 1-9

1 | 2 | 3
---------
4 | 5 | 6
---------
7 | 8 | 9

*/

// prompt for nodejs is required: https://github.com/flatiron/prompt
// npm install prompt
var prompt = require('prompt');

var board = {
    1: ' ',
    2: ' ',
    3: ' ',
    4: ' ',
    5: ' ',
    6: ' ',
    7: ' ',
    8: ' ',
    9: ' '
};

function markBoard(position, mark) {
    board[position] = mark.toUpperCase();
}

function printBoard() {
    console.log('\n' +
        ' ' + board[1] + ' | ' + board[2] + ' | ' + board[3] + '\n' +
        ' ---------\n' +
        ' ' + board[4] + ' | ' + board[5] + ' | ' + board[6] + '\n' +
        ' ---------\n' +
        ' ' + board[7] + ' | ' + board[8] + ' | ' + board[9] + '\n');

}

function isInt(value) {
    var x;
    if (isNaN(value)) {
        return false;
    }
    x = parseFloat(value);
    return (x | 0) === x;
}

function validateMove(position) {
    if (isInt(position) === true && board[position] === ' ') {
        return true;
    }
    return false;
}

// Everyone possible combination of three in a row
var winCombinations = [[1, 2, 3], [4, 5, 6], [7, 8, 9], [1, 4, 7],
                       [2, 5, 8], [3, 6, 9], [1, 5, 9], [3, 5, 7]];

// Determins if the passed in player has three in a row
function checkWin(player) {
    for (var i = 0; i < winCombinations.length; i++) {
        var markCount = 0;
        for (var j = 0; j < winCombinations[i].length; j++) {
            if (board[winCombinations[i][j]] === player) {
                markCount++;
            }
            if (markCount === 3) {
                return true;
            }
        }
    }
    return false;
}

function playTurn(player) {

    console.log('Your turn player: ' + player);
    prompt.start();
    prompt.get(['position'], function (err, result) {

        if (validateMove(result.position) === true) {
            markBoard(result.position, player);
            printBoard();
            if (checkWin(player) === true) {
                console.log('Winner Winner!');
                return;
            }
            if (player === 'X') {
                playTurn('O');
            } else {
                playTurn('X');
            }
        } else {
            console.log('incorrect input please try again...');
            playTurn(player);
        }
    });
}

console.log('Game started: \n' +
    ' 1 | 2 | 3 \n' +
    ' --------- \n' +
    ' 4 | 5 | 6 \n' +
    ' --------- \n' +
    ' 7 | 8 | 9 \n');

playTurn('X');

This code was very well organized and was extremely easy to follow along.

Looks to me that board is just a 1-based array.

It would be easier (and it makes more sense) to use an array, instead of an object.

But don't change how you are asking the user for input. You want to translate the user input one number back, so if the user enters "1" for box number 1, just subtract 1 from the integer version of the input and then pass it to markBoard.

if (validateMove(result.position) === true) {
    markBoard(result.position - 1, player);
    printBoard();

In your markBoard function, there is no point in calling .toUpperCase as the player variable passed in from playTurn is already a capital letter.

You are making validateMove too complicated. Just do:

return isInt(position) && board[position] === ' ';

If the expression evaluates to true, it will return true. If it evaluates to false, it will return false.

Note: I omitted the === true because JavaScript automatically checks for something to equal true.

It doesn't look like you have anything to check if the user enters an invalid number.

I would add something to the end of validateMove to check to make sure that the option is in bounds.

Here is what I came up with:

return isInt(position) && board[position] === ' ' && (position >= 0 && position <= 8);

It would be >= 1 && <= 9 if you continued to use the object instead of the array for storing squares of the board

This might help your efficiency:

Try moving this conditional from checkWin

if (markCount === 3) {
    return true;
}

Outside of the inner for loop so you aren't doing a conditional check with each iteration.

In checkWin, you do a lot of looping. In each loop, you are destroying and re-making variables as they exit and leave the scope (the loop).

This is very optimal. To help this, move the declarations of markCount, i, and j to the very top of the function (before the loops). You can set them to 0 when they are needed.

Here is what I am saying:

function checkWin(player) {
    var i, j, markCount
    for (i = 0; i < winCombinations.length; i++) {
        markCount = 0;
        for (j = 0; j < winCombinations[i].length; j++) {
            if (board[winCombinations[i][j]] === player) {
                markCount++;
            }
            if (markCount === 3) {
                return true;
            }
        }
    }
    return false;
}

Notice how I removed the vars?

I'm a little confused by what your isInt function is trying to accomplish.

To make it simpler, just run isNaN on the position. It will return NaN (which acts like false in a conditional) if the user entered a string, rather than a number.


Searching all object property values inside Array.prototype.filter callback Based on input from the UI (most likely a string) I would like to search a list of object and return all object that has one or more matching property values which match the given input.

Do you have any thoughts regarding performance or overall design?

The only requirement is that I need to keep the outer Array.prototype.filter. My question is how/if I should modify anything inside its callback.

var items = [{ x: 1, y: 2 }, { x:1, y: 3, z: 'blah' }];

function findObjectByPropertyValue(value, ignoreCase){

    var result = items.filter(function(item){

       var keys = Object.keys(item);
       for(var i = 0, len = keys.length; i < len; i++){

           var match,
               propertyValue = item[keys[i]];

           if(ignoreCase && typeof propertyValue === 'string' && typeof value === 'string')
               match = propertyValue.toLowerCase() === value.toLowerCase();
           else
               match = propertyValue === value;

           if(match)
               return true;
        }
    });

    return result;
}

console.assert(findObjectByPropertyValue(1).length === 2);
console.assert(findObjectByPropertyValue(2).length === 1);
console.assert(findObjectByPropertyValue(4).length === 0);
console.assert(findObjectByPropertyValue('blah').length === 1);
console.assert(findObjectByPropertyValue('Blah', true).length === 1);
console.assert(findObjectByPropertyValue('Blah').length === 0);

http://jsfiddle.net/Lg6rwnwh/1/

The method console.assert is a non-standard feature and it may not work across different user's browsers as each browser may have a different implementation, or may not have implemented it at all.

Read more at the red box at the top of this Mozilla Developer Network article.

I'd say you are better off using console.log because most browsers implement that, and use the same base implementation.

var keys = Object.keys(item);
for(var i = 0, len = keys.length; i < len; i++){

This is slightly re-inventing the wheel.

JavaScript already has a type of loop called for/in loop. It's syntax is this:

for(var key in object) {

}

Where key is a variable that will iterate through all the properties/keys of object. You could then access the value of that key/property by using

object[key]

withing the loop.

Instead of hard-coding the array items as the array that your function is checking, write over Array.prototype with this as a new method.

Then, you can refer to the array to check as this.

Here is roughly what I mean:

Array.prototype.findObjectByPropertyName(value, ignoreCase) {
    var result = this.filter(...);
    ...
}

Then, if you wanted to use this function on items, you would simply call (for example):

items.findObjectByPropertyName('blah', true);

if(ignoreCase && typeof propertyValue === 'string' && typeof value === 'string')
     match = propertyValue.toLowerCase() === value.toLowerCase();
 else
     match = propertyValue === value;

This is a debatable topic; many will say that you should always have {}s no matter how short the code inside the block will be.

I believe they say this because it greatly improves the readability of the organization of your code because all the code that is associated with, for example, that if statement is clearly seen in between the {}s.


Is this attempt at a merge sort correct? I am trying to write a merge sort algorithm. I can't tell if this is actually a canonical merge sort. If I knew how to calculate the runtime I would give that a go. Does anyone have any pointers?

public static void main(String[] argsv) {

    int[] A = {2, 4, 5, 7, 1, 2, 3, 6};
    int[] L, R;

    L = new int[A.length/2];
    R = new int[A.length/2];

    int i = 0, j = 0, k;

    PrintArray(A);
    // Make left and right arrays
    for (k = 0; k < A.length; k++) {
        if (k < A.length/2) {
            L[i] = A[k];
            i++;
        }
        else {
            R[j] = A[k];
            j++;
        }
    }

    PrintArray(L);
    PrintArray(R);

    i = 0;
    j = 0;
    // Merge the left and right arrays
    for (k = 0; k < A.length; k++) {
        System.out.println(i + " " + j + " " + k);
        if (i < L.length && j < R.length) {
            if (L[i] < R[j]) {
                PrintArray(A);
                A[k] = L[i];
                i++;
            }
            else {
                PrintArray(A);
                A[k] = R[j];
                j++;
            }
        }
    }

    PrintArray(L);
    PrintArray(R);
    PrintArray(A);
}

public static void PrintArray(int[] arrayToPrint) {
    for (int i = 0; i < arrayToPrint.length; i++) {
        System.out.print(arrayToPrint[i] + " ");
    }
    System.out.print("\n");
}

When a prospective employer asks you a question like this one, they most likely want to see something original.  They want you to show them that you can create code and not just copy and paste bits and pieces of code.

If what you wrote works and runs correctly, then you have accomplished the answer.

Sometimes employers are trying to find out if you know the theory behind a merge sort.  It looks to me like you have done that as well.

The only thing that I can think of that you could do to make your code better is to improve the naming of your variables.

int[] A = {2, 4, 5, 7, 1, 2, 3, 6};
int[] L, R;

L = new int[A.length/2];
R = new int[A.length/2];

Change them to

int[] array = {2, 4, 5, 7, 1, 2, 3, 6};
int[] leftSide;
int[] rightSide;

leftSide = new int[array.length/2];
rightSide = new int[array.length/2];

This would make it much easier to read.


Preventing SQL injection by converting all characters to their ASCII values In order to prevent SQL injection, I'm converting every character of a string to be inserted in the database into its ASCII value before performing the query. In order to read the value of the string from the database, I'm reversing the operation.

Is this method safe?

<?php
function toDatabase($string){
    $split = str_split(htmlspecialchars($string));
    $ascii = "";
    foreach($split as $letter){
        $ascii .= ord($letter).'-';
    }
    return $ascii;
}
function fromDatabase($string){
    $explode = explode("-",$string);
    $phrase = "";
    foreach($explode as $ascii_char){
        $phrase .= chr($ascii_char);
    }
    return $phrase;
}
$toBeInserted = toDatabase($_POST['comment']);
$connect = mysqli_connect("","","","");
$query = mysqli_query($connect,"INSERT INTO comments(comment) VALUES ('".$toBeInserted."')");
if(!$query){ die('Error!'); }
$fetch_query = mysqli_query($connect,"SELECT comment FROM comments");
if(!$fetch_query){ die('Error!'); }
while($assoc = mysqli_fetch_assoc($fetch_query)){
    echo fromDatabase($assoc['comment']).'<hr>';
}
?>

There's a common credo believed when evaluating the security of software:

If it's homemade, it's unlikely secure.

Sec.SE has a Q&A about homemade algorithms, which is somewhat germane to your circumstance.

I suggest you look into preparing your queries, as that would be your best action to take in this situation. It's essentially what you're trying to do anyways. Again, the PHP docs contain more information on mysqli::prepare.

I just noticed your comment to Mat's Mug, and prepared queries will not reach the database twice as you've said. When they're implemented correctly, they will be 100% secure (in doing their job, you might have to protect against other faults).

If you're worried about performance or efficiency, you might find the following select quotes helpful:

  Prepare is followed by execute. During execute the client binds parameter values and sends them to the server.

The database is not reached twice.

  A prepared statement can be executed repeatedly. Upon every execution the current value of the bound variable is evaluated and sent to the server. The statement is not parsed again. The statement template is not transferred to the server again.

I know you said it's only a single query per page, but things change in the future, and it's better to know you'll be safe then too.

  Prepared statements are using the so called binary protocol. The MySQL server sends result set data "as is" in binary format. Results are not serialized into strings before sending. The client libraries do not receive strings only. Instead, they will receive binary data and try to convert the values into appropriate PHP data types.

If you happen to be curious as to how and why paramterized queries are so much safer!

These quotes are from the PHP manual on prepared statements.


Finding the year using strtotime vs calculating manually I need to calculate at which year a school class has started, given the year they currently are in. For example a class of 2nd graders have started school in 2013, if I pass 2 to the function it should return 2013.

So, I need to take x years and 6 months out of a date, I was wondering which is the better way to do it.

Using strtotime():

function school_started1($years){
    return date('Y', strtotime('now - '.$years.' year - 6 month'));
}

Or calculating it manually:

function school_started2($years){
    $seconds_in_half_a_year = 365.25 * 43200;
    return date('Y', time() - $years * 365.25 * 86400 - $seconds_in_half_a_year);
}

I'm thinking the second method would be better because it doesn't have to do any parsing and stuff ( not quite a fan of parsing ).  I can do the math myself, are there any benefits/downsides to using strtotime?  What do you think?

The first version, using strtotime(), is definitely more maintainable.  The code obviously says exactly what you intend.  I like it.

The second version is full of magic numbers (even though they are somewhat recognizable magic numbers), and is therefore not as clear.  I don't like it, but if you wanted to use it anyway, I suggest:

function school_started2_alt($years){
    $SECONDS_PER_YEAR = 365.25 * 86400;
    return date('Y', time() - ($years + 0.5) * $SECONDS_PER_YEAR);
}

If you are using PHP ≥ 5.3, also consider this formulation, which flows nicely from left to right:

(new DateTime())->sub(new DateInterval("P${years}Y6M"))->format('Y');

The kinds of strings accepted by the DateInterval constructor follow a rigid format, so the parsing should be efficient. Keep in mind that PHP itself is an interpreted language, so the work to parse a DateInterval specifier string is insignificant relative to the total amount of parsing involved. You should therefore take full advantage of PHP's built-in features to simplify your own code.


Yet another BrainF*** Interpretter in Haskell I wrote a BrainF*** interpretter in Haskell. A general review would be helpful and especially, a suggestion with a better way to deal with the IO (IO Memory, IO ()) would be helpful.

import Data.Either
import Data.List
import Data.Char
import System.Environment
import System.IO

type Program = [Instruction]

data Instruction = Lft
            | Rgt
            | Inc
            | Dec
            | Nop
            | Loop Program
            | Out
            | Inp
            deriving Eq

data Memory = Memory [Int] [Int]

emptyMem :: Memory
emptyMem = Memory [] [0]

parse :: String -> Program
parse xs = go xs []
   where
      go :: String -> [Either Char Instruction] -> Program
      go "" is      = reverse $ rights is
      go ('[':xs) is   = go xs ((Left '['):is)
      go (']':xs) is   = go xs ((Right l):l2)
         where
            (l1, (_:l2)) =  span (/= Left '[') is
            l = Loop $ (rights.reverse) l1
      go ('>':xs) is   = go xs (Right Rgt:is)
      go ('<':xs) is   = go xs (Right Lft:is)
      go ('+':xs) is   = go xs (Right Inc:is)
      go ('-':xs) is   = go xs (Right Dec:is)
      go ('.':xs) is   = go xs (Right Out:is)
      go (',':xs) is   = go xs (Right Inp:is)

runInstruction :: Memory -> Instruction -> IO (IO Memory, IO ())
runInstruction mem Nop = return (return mem, return ())
runInstruction (Memory xs (y:[])) Rgt = return (return (Memory (y:xs) [0]), return ())
runInstruction (Memory xs (y:ys)) Rgt = return (return (Memory (y:xs) ys), return ())
runInstruction (Memory (x:xs) ys) Lft = return (return (Memory xs (x:ys)), return ())
runInstruction (Memory [] ys) Lft = return (return (Memory [] (0:ys)), return ())
runInstruction (Memory xs (y:ys)) Inc = return (return (Memory xs ((y+1):ys)), return ())
runInstruction (Memory xs (y:ys)) Dec = return (return (Memory xs ((y-1):ys)), return ())
runInstruction mem@(Memory _ (x:xs)) Out = return (return mem, putChar (chr x))
runInstruction (Memory xs (y:ys)) Inp = return (mem', return ())
                                          where mem' = getChar >>= \z -> return (Memory xs ((ord z):ys))
runInstruction mem@(Memory xs (0:ys)) (Loop _) = return (return mem, return ())
runInstruction mem (Loop prog) = do
                                    (mem1, act') <- runProgram mem prog
                                    mem' <- mem1
                                    (mem'', act'') <- runInstruction mem' (Loop prog)
                                    return (mem'', act' >> act'')

runProgram :: Memory -> Program -> IO (IO Memory, IO ())
runProgram mem [] = return (return mem, return ())
runProgram mem (i:is) = do
                           (mem1, act1) <- runInstruction mem i
                           mem' <- mem1
                           (mem2, act2) <- runProgram mem' is
                           return (mem2, act1 >> act2)

main :: IO ()
main = do
   hSetBuffering stdin NoBuffering
   (fileName:_) <- getArgs
   prog <- readFile fileName
   (mem, act) <- runProgram emptyMem (parse (filter (`elem` "[]+-><.,") prog))
   act

First of all, good job. Now, what can we improve? Let's start with the largest function first, runInstruction:

runInstruction :: Memory -> Instruction -> IO (IO Memory, IO ())
runInstruction mem Nop = return (return mem, return ())
runInstruction (Memory xs (y:[])) Rgt = return (return (Memory (y:xs) [0]), return ())
runInstruction (Memory xs (y:ys)) Rgt = return (return (Memory (y:xs) ys), return ())
runInstruction (Memory (x:xs) ys) Lft = return (return (Memory xs (x:ys)), return ())
runInstruction (Memory [] ys) Lft = return (return (Memory [] (0:ys)), return ())
runInstruction (Memory xs (y:ys)) Inc = return (return (Memory xs ((y+1):ys)), return ())
runInstruction (Memory xs (y:ys)) Dec = return (return (Memory xs ((y-1):ys)), return ())
runInstruction mem@(Memory _ (x:xs)) Out = return (return mem, putChar (chr x))
runInstruction (Memory xs (y:ys)) Inp = return (mem', return ())
                                          where mem' = getChar >>= \z -> return (Memory xs ((ord z):ys))
runInstruction mem@(Memory xs (0:ys)) (Loop _) = return (return mem, return ())
runInstruction mem (Loop prog) = do
                                    (mem1, act') <- runProgram mem prog
                                    mem' <- mem1
                                    (mem'', act'') <- runInstruction mem' (Loop prog)
                                    return (mem'', act' >> act'')

"Memory" handling

Almost all of those functions concern Memory, which isn't obvious. That's because your included all Memory functionality into runInstruction. If you follow that model, you can never use Memory in any other context.

So instead let us write some other functions:

moveRight   :: Memory -> Memory
moveLeft    :: Memory -> Memory
getValue    :: Memory -> Int
setValue    :: Int -> Memory -> Int
modifyValue :: (Int -> Int) -> Memory -> Memory

Instead of data Memory = Memory [Int] [Int], we're going to use

data Memory = Memory [Int] Int [Int]

That way we will always have a current value. The initial empty memory is the same as yours, except for the pivot value

emptyMemory = Memory [] 0 []

and moving left or right stays mostly the same:

moveRight (Memory []     v rs) = Memory [] 0 (v:rs)
moveRight (Memory (l:ls) v rs) = Memory ls l (v:rs)
moveLeft (Memory ls v [])     = Memory (v:ls) 0 []
moveLeft (Memory ls v (r:rs)) = Memory (v:ls) r rs

getValue (Memory _ v _ )       = v
setValue v                     = modifyValue (const v)
modifyValue f (Memory ls v rs) = Memory ls (f v) rs

What's nice about those functions is that we can easily check them with QuickCheck or other testing frameworks, since they are all pure. For example, we can check that

prop_set_get x mem = getValue (setValue x mem)   == x
prop_set_set x mem = setValue (setValue x mem)   == (setValue x mem)
prop_get_set   mem = setValue (getValue mem) mem == mem

This is hard to check with your current variant.

By the way your variant was completely opaque whether the left or the right list would provide the pivot element. Our new Memory makes sure that we cannot do it wrong, and that v in Memory _ v _ always exists.

How does runInstruction look with those changes?

runInstruction :: Memory -> Instruction -> IO (IO Memory, IO ())
runInstruction mem Nop = return (return mem, return ())
runInstruction mem Rgt = return (return (moveRight mem), return ())
runInstruction mem Lft = return (return (moveLeft  mem), return ())
runInstruction mem Inc = return (return (modifyValue succ mem), return ())
runInstruction mem Dec = return (return (modifyValue pred mem), return ())
runInstruction mem Out = return (return mem, putChar (chr (getValue mem)))
runInstruction mem Inp = return (mem'      , return ())
                           where mem' = getChar >>= \z -> return (setValue (const z) mem)
runInstruction mem (Loop prog)
  | getValue mem == 0 = return (return mem, return ())
  | otherwise         = do
                        (mem1, act') <- runProgram mem prog
                        mem' <- mem1
                        (mem'', act'') <- runInstruction mem' (Loop prog)
                        return (mem'', act' >> act'')

Almost readable, right? But something is still off.

Better types

Your result type of runInstruction doesn't really capture what you want to do. You want to run an instruction and get a new Memory, maybe with some output on your console, or some user input taken. But even without that information, we'll notice that you never user the IO () part of your result, except for putChar. That line could also get written as

runInstruction mem Out = putChar (getValue mem) >> return (return mem, return ())

Therefore we don't need it at all. So we could use IO (IO Memory) instead (spoiler: don't):

runInstruction :: Memory -> Instruction -> IO (IO Memory)
runInstruction mem Nop = return (return mem)
runInstruction mem Rgt = return (return (moveRight mem))
runInstruction mem Lft = return (return (moveLeft  mem))
runInstruction mem Inc = return (return (modifyValue succ mem))
... don't use this!

But we're stopping short before the finish line. As I said, you just want to change the memory in your program, and maybe, maybe want to show or take some text. So runInstruction's correct type would be:

runInstruction :: Instruction -> Memory -> IO Memory

It's implementation stays the same, except that we lose several returns:

runInstruction :: Instruction -> Memory -> IO Memory
runInstruction i mem@(Memory _ v _) = case i of
  Lft       -> return (moveLeft mem)
  Rgt       -> return (moveRight mem)
  Inc       -> return (modifyValue succ mem)
  Dec       -> return (modifyValue pred mem)
  Out       -> putChar v >> return mem
  Inp       -> (\z -> setValue (ord z) mem) `fmap` getChar
  Loop _ | v == 0 -> return mem
  Loop p    -> runProgram p mem >>= runInstruction i
  _         -> return mem

I've switched the order of Memory and Instruction so that both occurences of Memory are closer to each other. Of course, this means that we need to change runProgram too, but that change isn't really large:

runProgram :: Program -> Memory -> IO Memory
runProgram []     mem = return mem
runProgram (i:is) mem = runInstruction i mem >>= runProgram is

I think the last lines in both functions showed why I moved Memory to be the last argument.

You're off course free to change runInstruction back to your pattern matching variant.

Time for some exercises

Now that we have something that's easier to read, it would be time to refactor. So here are some exercises:

Brainfuck only supports integers from 0-255 officially, and it wraps. This should get reflected in our functions. With our new program structure, there is only one single point where you need to change something. Where? And why?
Instead of Nop, use Comment Char. That way, you can still have a look at the code.
Try to make a variant of runProgram that takes a String to simulate user input and produces a String to simulate output.
Try to parse more variants of Brainfuck, for example charmander, whitespace or Ook?!.


Distinct nearest pixels for floating-point coordinates dem_rows and dem_cols contain float values for a number of things i can identify in an image, but I need to get the nearest pixel for each of them, and than to make sure I only get the unique points, and no duplicates.

The problem is that this code is ugly and as far as I get it, as unpythonic as it gets. If there would be a pure-numpy-solution (without for-loops) that would be even better.

# next part is to make sure that we get the rounding done correctly, and than to get the integer part out of it
# without the annoying floatingpoint-error, and without duplicates
fielddic={}
for i in range(len(dem_rows)):
    # here comes the ugly part: abusing the fact that i overwrite dictionary keys if I get duplicates
    fielddic[int(round(dem_rows[i]) + 0.1), int(round(dem_cols[i]) + 0.1)] = None
# also very ugly: to make two arrays of integers out of the first and second part of the keys
field_rows = numpy.zeros((len(fielddic.keys())), int)
field_cols = numpy.zeros((len(fielddic.keys())), int)
for i, (r, c) in enumerate(fielddic.keys()):
    field_rows[i] = r
    field_cols[i] = c

I'm not sure what you are doing, exactly, but looping over range(len(...)) is never very pythonic. Since you only use the index to look up values, you should just take the pairs using zip.

Also, that round() + 0.1 thing is unnecessary. Floats represent all integers in their accurate area exactly. If you are outside the accurate area (i.e. an exponent larger than the number of mantissa bits), that addition will not change the value of the float either. Thus, you can simply cast the result of round() to an integer.

If you need to take the unique items of something, the best way is to not use a dict, but a set.

int_rows = map(lambda x: int(round(x)), dem_rows)
int_cols = map(lambda x: int(round(x)), dem_cols)
s = set(zip(int_rows, int_cols))

Then iterate over the set, or unzip it into two sequences for constructing the arrays.

Note: In python 2.x map and zip create lists, so you may want to use itertools.izip and imap. In python 3.x they are fine as is.


Composition or Private Inheritance for Implementing Builder? Over on StackOverflow, I was asked if I could come up with an example where private inheritance would be preferred to composition (in C++).  The following is the situation I described, and I was wondering which implementation you would prefer.  Most of the references I've found to private inheritance are poor uses, and I agree that it is rarely useful.

Let's say you have a class Foo that should be immutable, but it's rather complicated to set up (maybe you're reading settings from a file), so you decide to create a separate class FooBuilder to set up your Foo objects for you.  My solution (using private inheritance) is to declare an abstract interface:

class MutableFoo
{
  public:
     virtual void setValue(int n) = 0;
     virtual void setSomethingElse(string s) = 0;
};

And my original Foo class:

class Foo : private MutableFoo
{
  public:
     Foo(FooBuilder builder)
     {
         builder.build(this);
     }

     int value() const { return myValue; }
     string somethingElse() const { return mySomethingElse; }

  private:
     void setValue(int n) { myValue = n; }
     void setSomethingElse(string s) { mySomethingElse = s; }

     int myValue;
     string mySomethingElse;
};

And the FooBuilder:

class FooBuilder
{
   public:
     FooBuilder(string fileWithSettings);
     void build(MutableFoo *fooToBuild);
};

Now I can create read-only Foo objects without declaring them all to be const:

Foo aFoo(FooBuilder(aFileWithSettings));

A similar implementation using composition might look something like this:

class Foo
{
   public:
      Foo(FooData d);

      int value() const { return data.value(); }
      string somethingElse() const { return data.somethingElse(); }

   private:
      FooData data;
};

class FooData
{
  public:
    int value() const { return myValue; }
    int somethingElse() const { return mySomethingElse; }

    void setValue(int n) { myValue = n; }
    void setSomethingElse(string s) { mySomethingElse = s; }

  private:
    int myValue;
    string mySomethingElse;
};

class FooBuilder
{
  public:
    FooBuilder(string fileWithSettings);
    FooData getData() const;
};

Foo aFoo(FooBuilder(aFileWithSettings).getData());

I don't see much difference between these implementations as far as coupling is concerned (which is the complaint I see most frequently leveled against inheritance vs. composition).  In fact, the version using inheritance may have less coupling, as both Foo and FooBuilder depend on an abstract interface (MutableFoo) instead of a concrete class FooData.  Also, if we have lots of methods, the composition version will end up with lots of simple delegations, which is tedious if nothing else.

Which do you prefer?  Have I found a reasonable use for private inheritance?  Have you found other reasonable uses?

And finally, has anyone ever found a legitimate use for protected inheritance? :-P

Herb Sutter in his book 'Exceptional C++', Item 24 (Uses and Abuses of Inheritance), discusses the issue, and cites the following reasons for using private inheritance (instead of containment):

Override a virtual function
Access to a protected member
Construct the used object before, or destroy it after, another base sub-object
Share a common virtual base class or override the construction of a virtual base class
We benefit substantially from the empty base class optimization
We need "controlled polymorphism" - LSP IS-A, but in certain code only

If one of these situations applies, then you must use private inheritance.  Otherwise, containment is preferred.

The last of these reasons is cited as a possible reason for protected inheritance.

He also comments:

  That's as complete a list as I can make of the reasons to use non-public inheritance.  (In fact, just one additional point would make this a complete list of all reasons to use any kind of inheritance: We need public inheritance to express IS-A. [...])

The item occupies 8 pages.


Find minimum depth in a binary tree Description:

Given a binary tree, find its minimum depth.

The minimum depth is the number of nodes along the shortest path from the root node down to the nearest leaf node.

Note: A leaf is a node with no children.

Leetcode

Code:

class Solution {
    public int minDepth(TreeNode root) {
        if (root == null) return 0;
        return recur(root, 1);
    }

    private int recur(TreeNode root, int depth) {
        if (root == null) return 0;
        if (root.left == null && root.right == null) {
            return depth;
        }
        int ldepth = recur(root.left, depth + 1);
        int rdepth = recur(root.right, depth + 1);

        // either left or right subtree was not a leaf node
        // ignore non leaf node.
        if (ldepth == 0 || rdepth == 0) {
            return Math.max(ldepth, rdepth);
        }
        else {
            return Math.min(ldepth, rdepth);
        }
    }
}

Question:

I need to know if the given logic can be simplified?

If by simplification you mean optimization, then only one thing came to my mind:

Whenever you find a leaf node, remember its depth in minmalFound variable.
When again you hit the next leaf node you compare its depth with previously remembered node. And save the minimum of both into minmalFound.

And the main logic to add is whenever you are searching a certain path and find out that the depth becomes as great as the existing minmalFound, then you stop searching that path.

Here is code to demonstrate this technique:

public class Solution {
    public static int minDepth(TreeNode root) {
        reccallscount = 0;
        minmalFound = -1;
        if (root == null)
            return 0;
        return recur(root, 1);
    }

    public static boolean doFaster = false;
    private static int minmalFound = -1;
    private static int reccallscount = 0;

    private static int recur(TreeNode root, int depth) {
        reccallscount++;
        if (root == null) return 0;
        if (root.left == null && root.right == null) {
            if (minmalFound == -1) {
                minmalFound = depth;
            } else {
                minmalFound = Math.min(minmalFound, depth);
            }
            return depth;
        }

        if (doFaster) {
            if (minmalFound != -1 && depth + 1 >= minmalFound) {
                return minmalFound * 2;
            }
        }

        int ldepth = recur(root.left, depth + 1);
        int rdepth = recur(root.right, depth + 1);

        // either left or right subtree was not a leaf node
        // ignore non leaf node.
        if (ldepth == 0 || rdepth == 0) {
            if (ldepth != 0) {
                minmalFound = Math.min(minmalFound, ldepth);
            } else if (rdepth != 0) {
                minmalFound = Math.min(minmalFound, rdepth);
            }
            return Math.max(ldepth, rdepth);
        }
        else {
            int value = Math.min(ldepth, rdepth);
            minmalFound = Math.min(minmalFound, value);
            return value;
        }
    }

    public static void main(String args[]) {
        TreeNode root = new TreeNode();
        root.left = new TreeNode();
        root.left.right = new TreeNode();
        root.right = new TreeNode();
        root.right.left = new TreeNode();
        root.right.left.right = new TreeNode();
        root.right.left.right.left = new TreeNode();

        Solution.doFaster = false;
        int min = minDepth(root);
        System.out.println("Minimum " + min + " found with " + reccallscount + " recursive calls");

        Solution.doFaster = true;
        min = minDepth(root);
        System.out.println("Minimum " + min + " found with " + reccallscount + " recursive calls with faster approach");
    }
}

class TreeNode {
    TreeNode left = null;
    TreeNode right = null;
}

In the code above, I created a tree with left side depth = 3 and right side depth = 5.  I ran minDepth() twice; first using your algorithm, and again with my addition. The result is then printed:

Minimum 3 found with 11 recursive calls

Minimum 3 found with 5 recursive calls with faster approach


JavaScript modules loading with eval() I recently asked to use eval() for a module function. Now I ended up doing a very weird function which does what I want. I like the idea I made it but I'm not so sure if this is state of the art, or more, good to build an application on.

index.html:

<script src="module.js"></script>

<script>

require('alert');

bam(); // 9
</script>

module.js:

(function () {

    var require = window.require = function (fnName) {

        var dependencies = fnName.split(' '); // load multiple dependencies etc. (not nessecairy now)

        // REAL CASE WOULD BE, TO LOAD A .JS FILE INTO A STRING VAR
        var code = 'var bam = function() { var x = 1; var y = 2; var z = (x+y)*3; alert(z); }';

        return (function () {
            window.eval(code);
        })();
    }
})();

Is this a good solution for a require function like in PHP if I only load and eval() files made by me?

The functionality you are looking for is found in RequireJS and follows a spec called Asynchronous Module Definition (AMD). It is a mechanism that allows dependency management in JS, most notably in browsers.

As to why you think your own code is better, think again:

  it provides better latency on 3G network

It's because you think the modules are deployed "as is", as file-per-module basis. You are thinking of the "additional HTTP request per module" am I right?

Sure, they work when broken down into modules. But you don't actually deploy them that way. You use the optimier which minifies your modules, and combines related modules into a file. This results in:

Smaller files due to minification
Lesser files due to dependency combination
Lesser files to download since your dependencies are grouped into related modules.

Also, the modules are cached once loaded. Once another modules requires a module that is already loaded by another module earlier in the execution of your code, it uses the cached one rather than requesting it again.

  I don't like unnecessary function wrappers... for every module, every require call.

A require/define call is designed to:

define the dependencies
define the module
the filename is the module name

It isn't that hard to code:

//MyModule.js
define([
  'what',
  'you',
  'need'
],function(what,you,need){

  //what you do

  return whatYouWantExposedFromThisModule;

});

  I am now able to use my script like php's include.

Handling the modules

include is like appending one file to the other, which is synonymous to concatenating files. In JS, we don't want globals. What if one module of yours had a bam variable, but has bam in another and you required the two. How would you handle that?  Also, JS execution does not halt. How would you expect to do a require in your implementation? Synchronous XHR? preloading? That takes time! You'd be halting the execution for each require call of your implementation just to load a script.

<script>

  require('alert');
  //halt here to load alert?
  //you'd block the UI, freeze the browser, lose the user
  bam(); // 9

</script>

In the module definition in require, the code of the module lives in it's own scope. Nothing is spilled to the global scope. What is exposed by the module is only exposed to the function that requires it, which is also a closed scope. Also, modules are cached. Once a module is already loaded, future requires to that module are taken from the cache, not from the network. They are asynchronous, and does not block the UI thread and the requiring code only executes when the dependencies are loaded - nothing is halted while we wait for the dependencies to load.

Loading the modules

Suppose you will do AJAX with your eval approach - it's not cross-domain friendly, and cross-browser friendly (IE).

The concept of loading a module is based on script-loaders which can do cross-domain. It's similar to how JSONP works, but instead of an arbitrary callback, they are fed through define.

I suggest you read more about how script loaders and dependency loaders work. This is a broad topic and has it's own spec.


Best practice for properties initialization using constructor I am declaring a "class" using a "constructor method". In both samples, properties uuid and dateCreated are initialized but in different places.

I would like to know the "best practice" in this scenario and why (also in terms of readability).

Solution A

define([
    'dojo/_base/declare',
    'dojo/_base/lang',
], function (declare, lang) {
    return declare('xxx.xxx.Command', null, {
        uuid: utilis.UUID(), // IS GOOD PRACTICE?
        dateCreated: Date.now(), // IS GOOD PRACTICE?
        action: null,
        properties: null
        },
        constructor: function (action, receiver, properties) {
            this.action = action;
            this.receiver = String(receiver);
            this.properties = properties;
        }
    });
});

Solution B

define([
    'dojo/_base/declare',
    'dojo/_base/lang',
], function (declare, lang) {
    return declare('xxx.xxx.Command', null, {
        uuid: null,
        dateCreated: null,
        action: null,
        properties: null
        },
        constructor: function (action, receiver, properties) {
            this.uuid = utilis.UUID();
            this.dateCreated = Date.now();
            this.action = action;
            this.receiver = String(receiver);
            this.properties = properties;
        }
    });
});

I know very little about Dojo, but for me it makes sense to keep constructor as small as possible. I wouldn't also predefine properties with null values and go with this:

define([
    'dojo/_base/declare',
    'dojo/_base/lang',
], function (declare, lang) {
    return declare('xxx.xxx.Command', null, {
        uuid: utilis.UUID(),
        dateCreated: Date.now(),
        },
        constructor: function (action, receiver, properties) {
            this.action = action;
            this.receiver = String(receiver);
            this.properties = properties;
        }
    });
});


Checking if a multi dimensional array's given property is empty or not for the whole collection This is what I am doing and it's working:

var isEmpty = true;
for(let i = 0; i< members.length; i++)
{
    var member = members[i];
    if(member && member[3]){
        isEmpty = false;
        break;
    }
}

if(isEmpty) {
    var somePrefix = "123 -"
    for(let i = 0; i< members.length; i++)
    {
        var member = members[i];
        if(member && member[2]){
            member[3] = somePrefix + i;
        }
    }
}

Can this be improved? I'm simply checking if a specific property is empty for all of the collection of multi dimensional array.

Here is what's happening:

Checking X property is empty for all members
If it is, then auto populate it

If any of the property is filled in then leave it as is

Style

Some points on your style

Don't forget to close the line with semicolons where appropriate.
Use const for variables that do not change.
Use let for variables that are scoped to the block.
Use for of loops when you do not need the index.
In JS the opening block delimiter { is on the same line as the associated statement. eg for(...) { // << { on same line
Even if its an example, always write code as functions, not as flat bits of code as in your example. The function name adds additional contextual semantic meaning and forces you to write in a style more akin to real world needs.
Use a space between

tokens and (, eg for (, if (foo) {.
operators and operands, eg 1 + 2, i ++
commas and expressions, eg a, b  (not that it applies in your example)

Typescript?

You have the question tagged typescript, yet the code is pure JS. If you wanted a typescript version you have not indicated, and personally apart from better IDE integration the additional complexity of typescript does not offer any benefit over well written JS.

Rewrites

With all things equal the best quality source code has the shortest line count.

Cleaning up of your code

Note that because the code is written as a function I have not needed to use an intermediate to hold the abstract "empty" state of members, removing 4 lines of code.

function populateIfMembersEmpty(members, prefix = "123 -") {
    for (const member of members) {
        if (member && member[3]) { return }
    }
    for (let i = 0; i< members.length; i++) {
        if (members[i] && members[i][2]) { members[i][3] = prefix + i }
    }
}

Alternatives

function populateIfMembersEmpty(members, prefix = "123 -") {
    if (! members.some(member => member && member[3])) {
        members.forEach((member, i) => {
            if (member && member[2]) { member[3] = prefix + i }
        });
    }
}

or

Does a member of members on the same line need to be named, or is a symbolic representation any less meaningful?

function populateIfMembersEmpty(members, prefix = "123 -") {
    if (! members.some(m => m && m[3])) {
        members.forEach((m, i) => m && m[2] && (m[3] = prefix + i));
    }
}

or

function populateIfMembersEmpty(members, prefix = "123 -") {
    if (members.some(m => m && m[3])) { return }
    members.forEach((m, i) => m && m[2] && (m[3] = prefix + i));
}

19 lines down to 4 may sound pedantic, but apply the same to a large code base and a monster source file of 10,000 lines can be a far more manageable 2,000 reducing the odds of a bug by 80%

As the only response you have so far got that has unnecessarily emphasized a confusing point, I will say your style is neither "very" hard (or hard) to read, nor is it uncommon. (Let alone "very" uncommon, meaning... unique? no! ).