"Backwards Read Primes" of Codewars powershell version Link to problem: https://www.codewars.com/kata/5539fecef69c483c5a000015/train/powershell
I have a working solution in Codewars but my solution is slow for some reason and I need to improve the efficiency.
Please look over my solution and offer any suggestions on things I could do to improve what I have.
Instructions for problem:
Backwards Read Primes are primes that when read backwards in base 10 (from rightto left) are a different prime. (This rules out primes which are palindromes.)
Examples:
13 17 31 37 71 73 are Backwards Read Primes
13 is such because it's prime and read from right to left writes 31 which is prime too. Same for the others.
Task
Find all Backwards Read Primes between two positive given numbers (both inclusive), the second one always being greater than or equal to the first one. The resulting array or the resulting string will be ordered following the natural order of the prime numbers.
Example
backwardsPrime(2, 100) => [13, 17, 31, 37, 71, 73, 79, 97] backwardsPrime(9900, 10000) => [9923, 9931, 9941, 9967] backwardsPrime(501, 599) => []
Note for Forth
Return only the first backwards-read prime between start and end or 0 if you don't find any
Don't use Ruby Prime class, it's disabled.
backwardsPrime(2, 100) => [13, 17, 31, 37, 71, 73, 79, 97]
backwardsPrime(9900, 10000) => [9923, 9931, 9941, 9967]
Thanks.

$primes = @{ 2 = 2; 3 = 3; 5 = 5; 7 = 7; 11 = 11; 13 = 13; 17 = 17; 31 = 31; 37 = 37; 71 = 71; 73 = 73; 79 = 79; 97 = 97; 107 = 107; 113 = 113; 149 = 149; 157 = 157; 167 = 167; 179 = 179; 199 = 199 };

function backwards-prime($start, $stop)
{

    function revNum ([int] $n) {
        $numStr = $n.ToString();
        return ($numStr[-1..-($numStr.Length)] -join "") -as [int];
    }

    function isPrime ($num) {

        if ($primes.containsKey($num)) {
            return $true;
        }

        if ($num -lt 3) {
            return $num -gt 1;
        }

        if ($num % 2 -eq 0 -or $num % 3 -eq 0) {
            return $false;
        }

        $i = 5;

        while ($i * $i -le $num) {
            if ($num % $i -eq 0 -or $num % ($i + 2) -eq 0) {
                return $false;
            }

            $i += 6;
        }

        $primes[$num.ToString()] = $num;

        return $true;
    }

    $res = @();

    if($start % 2 -eq 0) { $start = $start + 1; }
    if($stop % 2 -eq 0) { $stop = $stop - 1; }

    for ($i = $start; $i -le $stop; $i += 2) {
        $nPrime = isPrime $i;
        $nReversed = revNum $i;
        $nPrimeReverse = isPrime $nReversed;

        if ($nPrime -and $nPrimeReverse -and $i -ne $nReversed -and $i -ge 13) {
            $res += $i;
        }
    }

    return ($res -join ", ").Trim();
}

Update: Later this night after I asked my question I continued to think of ways I could have improved the performance of my solution. I made some refactors that I'm going to share which gave me enough boost to beat the time requirement for this problem.
But please continue to post your answers, I'm here to learn as well. And any new nugget of efficiency I can add or look at is still super valuable to me.
Thanks in advance...
So the fist changes I made even though a bit small was
I removed the .ToString() to my $num in my isPrime function, I realized that it was not even needed, and yes that's a small change but every bit helps in this case...
The update isPrime function below...

  function isPrime ($num) {

      if ($primes.containsKey($num)) {
          return $true;
      }

      if ($num -lt 3) {
          return $num -gt 1;
      }

      if ($num % 2 -eq 0 -or $num % 3 -eq 0) {
          return $false;
      }

      $i = 5;

      while ($i * $i -le $num) {
          if ($num % $i -eq 0 -or $num % ($i + 2) -eq 0) {
              return $false;
          }

          $i += 6;
      }

      $primes[$num] = $num; # removed the .ToString() here... very small optimization but it all matters I guess.

      return $true;
  }

Next I went to the for-loop and realized that I was doing 2 isPrime calls and a reverse number call as well... It dawned on me that I could easily just declare and assign $nPrime variable and then check if it's false then no need to continue and just go to the next iteration, same with $nPrimeReverse. But before that I would call revNum and test if $nReversed is equal to $i if it is, then also I would call continue and start the next iteration. Then finally I just add a check at the start determining if $i is greater than 13.
Here is the updated for-loop:

  for ($i = $start; $i -le $stop; $i += 2) {

      if ($i -lt 13) { continue; }

      $nPrime = isPrime $i;

      if ($nPrime -eq $false) { continue; }

      $nReversed = revNum $i;

      if ($nReversed -eq $i) { continue; }

      $nPrimeReverse = isPrime $nReversed;

      if ($nPrimeReverse -eq $false) { continue; }

      $res += $i;
  }

After all these small changes my solution passed within the time constraint.
Please feel free to comment any additional items that I could refactor and improve.
Warm Regards


Time complexity of tape equilibirum As per the instructions on this Tape Equlibrium-Codility

I've written the code

import java.util.Arrays;
public class TapeEqulibrium {
  int main(int[] array) {
    int minimumDiff = 100000;
    for (int i = 1; i < array.length; i++) {
        int differnce = Math.abs(Arrays.stream(array, 0, i).sum() - Arrays.stream(array, i, array.length).sum());
        minimumDiff = differnce < minimumDiff ? differnce : minimumDiff;
    }
    return minimumDiff;
   }
}

On submission, I have got correct answers for all test cases. But It says that the time complexity is O(n*n)and taking more than 6.00 seconds. while writing I thought that it falls under time complexity of O(n) or something (as I don't know much about that). So I need to know how to calculate the time complexity of this code.

You inner loop contains:

int differnce = Math.abs(Arrays.stream(array, 0, i).sum() - Arrays.stream(array, i, array.length).sum());

That does (i-0) operations for first sum and (array.length-i) for second, for a total of array.length-i+i-0=array.length. As the size of input is n=array.length that really gives O(n*n). Replacing for-loops with Streams doesn't improve complexity, only hides it a bit.


Suffixing numeric values This is a part of my Factorio mod Visual Signals.

This code takes a numeric value as input and returns a string for the value, for example:

-12 --> -12
1 --> 1
50 --> 50
999 --> 999
1500 --> 1.5k
24999 --> 24.9k
123456 --> 123k
1234567 --> 1.2M
12345678 --> 12M
123456789 --> 123M
1234567890 --> 1.2G
and so on...

The idea is to replicate the same way Factorio show signals in the circuit network.

The code works by determining a prefix, which simply is a potential minus sign. It then determines the middle part (the numbers and potential comma separator to use) and the suffix (the letter at the end).

The code seems to work perfectly fine and now I simply wonder: Can I improve this code somehow?

local suffixChars = { "", "k", "M", "G", "T", "P", "E" }

function CountString(count)
  local absValue = math.abs(count)
  local prefix = ""
  if count < 0 then
    prefix = "-"
  end
  local suffix = 1
  while absValue >= 1000 do
    absValue = absValue / 1000
    suffix = suffix + 1
  end

  local str = tostring(absValue)
  if absValue < 10 then
    return prefix .. string.sub(str, 1, 3) .. suffixChars[suffix]
  end
  if absValue < 100 then
    return prefix .. string.sub(str, 1, 2) .. suffixChars[suffix]
  end
  return prefix .. string.sub(str, 1, 3) .. suffixChars[suffix]
end

First off, your naming convention. You have a function name CountString, which to me states that you are either counting a string (?) or something along those lines. A better name would be FriendlyNumber or HumaneNumber.

count in a similar manner, means tracking an iterator. It could become input, rawValue etc.

Now, to the actual function. You can make use of a log function to compute your suffix (base 10), and the same for referring to your index in suffix list, as well as getting the absValue.

Instead of concatenating different parts of string, use a string.format, which would also remove your call to tostring.

Lua has a ternary operator, which can be done using and..or statements. Based on your version of lua engine (4.x, 5.{1, 2, 3}), you might have different implementation of the log function. A sample code would be:

-- For Lua 5.1
local suffixChars = { "", "k", "M", "G", "T", "P", "E" }
local l, f, a = math.log10, math.floor, math.abs  -- Lua 5.2+ have math.log(x, base)

function FriendlyNumber(rawValue)
  local absValue = a(rawValue)
  local prefix = rawValue < 0 and "-" or ""
  local log = f(l(absValue) / 3)
  -- I was calculating log first, but I needed floor(1/3rd) of it,
  -- and later had to multiply by 3 again, so computing the 1/3rd here only
  local suffix = suffixChars[1 + log]
  absValue = absValue / (10 ^ (log * 3))
  local str = ("%%.%df"):format((absValue < 1000 and absValue > 10) and 0 or 1):format(absValue)
  return ("%s%s%s"):format(prefix, str, suffix)
end

Working sample (and output):

-12 -12
1.0 1
50  50
999 999
1.5k    1500
25k 24999
123k    123456
1.2M    1234567
12M 12345678
123M    123456789
1.2G    1234567890


View model using different datetime component Any advice on how to make this code; cleaner, more effective, just overall better!
Program setup object that is coming from the database EF. It then maps the object to a view model and converts datetime to string and different datetime components.
//EF Data
            List<MeetingEvent> Meeting = new List<MeetingEvent>();
            Meeting.Add(new MeetingEvent() { MeetingId = 1, MeetingName= "A1",StartDateTime= new DateTime(2020, 1, 01),EndDateTime = new DateTime(2020, 1, 05) });
            Meeting.Add(new MeetingEvent() { MeetingId = 2, MeetingName = "A2", StartDateTime = new DateTime(2020, 2, 01), EndDateTime = new DateTime(2020, 2, 05) });
            Meeting.Add(new MeetingEvent() { MeetingId = 3, MeetingName = "A3", StartDateTime = new DateTime(2020, 3, 01), EndDateTime = new DateTime(2020, 3, 05) });
            Meeting.Add(new MeetingEvent() { MeetingId = 4, MeetingName = "A4", StartDateTime = new DateTime(2020, 4, 01), EndDateTime = new DateTime(2020, 4, 05) });
            Meeting.Add(new MeetingEvent() { MeetingId = 5, MeetingName = "A5", StartDateTime = new DateTime(2020, 5, 01), EndDateTime = new DateTime(2020, 5, 05) });
//Logic
            var listOfEvents = Meeting.Select(x => new MeetingEventViewModel
            {
                MeetingId = x.MeetingId,
                MeetingName = x.MeetingName,
                StartDateTime = x.StartDateTime,
                EndDateTime = x.EndDateTime,
                StartDateDayName = x.StartDateTime.DayOfWeek.ToString(),
                StartDateMonth = x.StartDateTime.Month.ToString(),
                StartDateDay = x.StartDateTime.Day.ToString(),
                StartDateYear = x.StartDateTime.Year.ToString(),
                EndDateDayName = x.EndDateTime.DayOfWeek.ToString(),
                EndDateMonth = x.EndDateTime.Month.ToString(),
                EndDateDay = x.EndDateTime.Day.ToString(),
                EndDateYear = x.EndDateTime.Year.ToString(),
            }).ToList().OrderBy(o => o.StartDateTime);

            foreach (MeetingEventViewModel m in listOfEvents)
            {
                Console.WriteLine(m.MeetingId);
            }

    class MeetingEvent
    {
        public int MeetingId { get; set; }
        public string MeetingName { get; set; }
        public DateTime StartDateTime{ get; set; }
        public DateTime EndDateTime { get; set; }
    }

    class MeetingEventViewModel
    {
        public int MeetingId { get; set; }
        public string MeetingName { get; set; }
        public DateTime StartDateTime { get; set; }
        public DateTime EndDateTime { get; set; }
        public string StartDateDayName { get; set; }
        public string StartDateDay { get; set; }
        public string StartDateYear { get; set; }
        public string StartDateMonth { get; set; }
        public string EndDateDayName { get; set; }
        public string EndDateDay { get; set; }
        public string EndDateYear { get; set; }
        public string EndDateMonth { get; set; }

}

First of all I would advice to use DateTimeOffset instead of DateTime.

Use the time, date, datetime2 and datetimeoffset data types for new work. These types align with the SQL Standard. They are more portable. time, datetime2 and datetimeoffset provide more seconds precision. datetimeoffset provides time zone support for globally deployed applications.
Source: https://docs.microsoft.com/en-us/sql/t-sql/data-types/datetime-transact-sql?view=sql-server-ver15

Besides that; If you have to do a lot of mapping I would use AutoMapper.
The mapping profiles (where you can do some mapping configuration) would look like this:
  public class MeetingEventProfile : Profile
  {
    public MeetingEventProfile()
    {
        CreateMap<MeetingEvent, MeetingEventViewModel>()
            .ForMember(x => x.StartDateDayName, x => x.MapFrom(_ => _.StartDateTime.DayOfWeek))
            .ForMember(x => x.StartDateMonth, x => x.MapFrom(_ => _.StartDateTime.Month))
            .ForMember(x => x.StartDateDay, x => x.MapFrom(_ => _.StartDateTime.Day))
            .ForMember(x => x.StartDateYear, x => x.MapFrom(_ => _.StartDateTime.Year))
            .ForMember(x => x.EndDateDayName, x => x.MapFrom(_ => _.EndDateTime.DayOfWeek))
            .ForMember(x => x.EndDateMonth, x => x.MapFrom(_ => _.EndDateTime.Month))
            .ForMember(x => x.EndDateDay, x => x.MapFrom(_ => _.EndDateTime.Day))
            .ForMember(x => x.EndDateYear, x => x.MapFrom(_ => _.EndDateTime.Year));
    }
  }

You only see some specific mappings for properties that don't match by name (or require some conversion/etc). Property names that match you don't have to explicitly map; although there is a lot of configuration possible here!
In your logic (which is not poluted with mapping anymore) you would do something like this:
public class MyClass{
  private readonly IMapper _mapper;

  public MyClass(IMapper mapper){
    _mapper = mapper;
  }

  public void MyMethod(){
    var meetingVms = _mapper.Map<MeetingEventViewModel[]>(Meeting);
  }
}

The performance impact by using AutoMapper is very low; I did quite some benchmarks with high performance requirements and the impact was very low.
My example assumes you are using dependency injection. AutoMapper is very easy to implement. Otherwise you have to create the Mapper instance yourself which isn't hard too. The docs are well written: https://docs.automapper.org/en/stable/Dependency-injection.html


Better understanding of unit testing in VBA from non-returning functions I have always wanted to start unit testing my code, and ending last week I downloaded Rubberduck again (first time there was just too little information how to do it to get me going) but after watching this video unit testing, I decided that it looks easy enough.

So the whole weekend I read up more and watched more videos and got more excited - because Monday I am going to go through my latest project and start incorporating unit tests!!

Or maybe not... I got to this first class of mine, and I really do not see a lot of places that I can do unit testing - meaning not a lot of return functions with a value to test.

The long and short of this class is that I inherited a worksheet with a lot of data all over the place, so much so that I can not even add or delete a line without breaking a lot of formulas (true story). Anyway, I made a new sheet, loaded information from a DB into that sheet, and from there on I am doing a data transfer to the infected patient (as mentioned above) every time I need to update the information.

My class with methods is below and I am wondering if there is anyway to run a test to check if data are transferred correctly, or to re-write the code to make it less brittle. Please feel free to tear the code apart and give any other advice you deem necessary.

Option Explicit
Private firstTruckColm As Long
Private truckRow As Long
Private numberOfShops As Long
Private numberOfProducts As Long
Private wb As Workbook
Private laailys As Worksheet
Private bloem As Worksheet
Private bloemShopNamesStartRow As Long
Private bloemShopNamesStartColm As Long

Public Sub loadTrucksInit()

    Call initValues
    Call insertShopNamesAndProd

End Sub

Private Sub initValues() 'Loads values from a "settings" module. (Any thoughts on this)'

    firstTruckColm = mdl_settings.returnLaailysFirstTruckColm
    truckRow = mdl_settings.returnLaailysTruckRow
    numberOfShops = mdl_settings.returnNumberOfBloemShops
    numberOfProducts = mdl_settings.returnNumberOfBloemProducts
    Set wb = ThisWorkbook
    Set laailys = wb.Sheets("Laailys")
    Set bloem = wb.Sheets("Bloem")
    bloemShopNamesStartRow = mdl_settings.returnBloemShopNamesStartRow
    bloemShopNamesStartColm = mdl_settings.returnBloemShopNamesStartColm

End Sub

Private Sub insertShopNamesAndProd()
    Dim a As Long
    For a = 1 To numberOfShops
        With laailys
            .Cells(truckRow, firstTruckColm + a - 1).Value = bloem.Cells(bloemShopNamesStartRow, bloemShopNamesStartColm + a).Value
        End With
        Call goThroughProducts(bloemShopNamesStartColm + a, firstTruckColm + a - 1)
    Next a
End Sub

Private Sub goThroughProducts(colmToUseBloem As Long, colmToUseLaailys As Long)
    Dim a As Long
    For a = bloemShopNamesStartRow + 1 To numberOfProducts + bloemShopNamesStartRow
        If bloem.Cells(a, colmToUseBloem).Value <> vbNullString Then
            Call insertProducts(bloem.Cells(a, 1).Value, bloem.Cells(a, colmToUseBloem).Value, colmToUseLaailys)
        End If
    Next a
End Sub

Private Sub insertProducts(productCode, prodAmount, colmToUseLaailys)
    Dim a As Long
    Dim checker As Boolean
    checker = False
    For a = 1 To 200
        If laailys.Cells(a, 4).Value = productCode Then
            laailys.Cells(a, colmToUseLaailys).Value = prodAmount
            checker = True
            Exit Sub
        End If
    Next a
    If checker = False Then
        MsgBox productCode & " did not read in correctly, make sure the product code in 'Laailys' is the same as in 'Bloem'"
    End If
End Sub

A few other questions that I also want to know if possible:

I like to have a initValues method that loads all the properties that I am going to use in my class all at once in one neat place if I need to change any. Is this an acceptable practice?
I point to colm values like firstTruckColm + a - 1, should I first declare another variable and give this value to that variable, just so that my arguments for other procedures look better?
Should I rather change my Subs to Functions with a return value as most I can?

You say this is a class, but you don't have anything exposed from it. What I mean is if I create an instance of this class, there's nothing I can do with it. I can't Get or Let any properties and I can't use any of the Private methods.

Call

You don't need to Call Sub anymore. You also don't need to enclose your arguments. E.g.

goThroughProducts somecolumn + a, somecolumn + a - 1

Worksheets

Worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid Sheets("mySheet") and instead just use mySheet.

Naming

Maybe some of this isn't in English and so I don't understand the meanings, but your naming could be improved. I have no idea what a laailys worksheet could be. Also you use product and prod both. Try to stay consistent and clear with your naming. What is checker checking? It's boolean so normally you would name it something like isMatch.

Also a Boolean is initialized as False so no need to set it as false.

Class

It seems strange to have a class refer to ThisWorkbook. What is your goal with this class? I don't see anything that can't be done with already existing objects.

ByVal

All your arguments are being passed ByRef implicitly. You want to pass ByVal whenever possible. e.g.

Private Sub InsertProducts(ByVal productCode As Long, ByVal productAmount As Long, ByVal targetColumn As Long

Typing

You'll notice I gave your variables a type above. Otherwise they are implicitly variants, which are not needed.

Loop

In InsertShopNamesAndProd you have a call to a procedure within a loop and a with:

For a = 1 To numberOfShops
        With laailys
            .Cells(truckRow, firstTruckColm + a - 1).Value = bloem.Cells(bloemShopNamesStartRow, bloemShopNamesStartColm + a).Value
        End With
        Call goThroughProducts(bloemShopNamesStartColm + a, firstTruckColm + a - 1)
    Next a

And in that procedure you have another call within a loop -

For a = bloemShopNamesStartRow + 1 To numberOfProducts + bloemShopNamesStartRow
        If bloem.Cells(a, colmToUseBloem).Value <> vbNullString Then
            Call insertProducts(bloem.Cells(a, 1).Value, bloem.Cells(a, colmToUseBloem).Value, colmToUseLaailys)
        End If
    Next a

That seems pretty repetitive. Instead think about refactoring these procedures so that you take all the products into an array, go through your shops and insert products as you work.

If

Here

If bloem.Cells(a, colmToUseBloem).Value <> vbNullString Then
            Call insertProducts(bloem.Cells(a, 1).Value, bloem.Cells(a, colmToUseBloem).Value, colmToUseLaailys)
        End If

Can be put on one line. And here

Dim checker As Boolean
    checker = False
    For a = 1 To 200
        If laailys.Cells(a, 4).Value = productCode Then
            laailys.Cells(a, colmToUseLaailys).Value = prodAmount
            checker = True
            Exit Sub
        End If
    Next a
    If checker = False Then
        MsgBox productCode & " did not read in correctly, make sure the product code in 'Laailys' is the same as in 'Bloem'"
    End If

I'm not sure how you can get to that second If without checker being false.

You can also use a boolean in an if as the actual test:

If Not checker Then

Settings module

You use your "settings" module to get objects and rows? That doesn't sound right. Settings should be loaded and stored so they can be reset. What you're doing is populating so naming it settings isn't entirely accurate.


Function to build a string based on the recursion depth I was doing a JavaScript challenge to test myself and I succeeded in finding a solution but the code is very ugly.  I was hoping to get some opinions on what a better solution would look like.

The problem is this.  We need to write, in Javascript, a function with signature

f(input)

that when called in the following ways give the following outputs

f("a") => "fa"
f()("a") => "foa"
f()()("a") => "fooa"

and so on, with the number of 'o''s being equal to the number of empty parentheses.

Here is my solution

function f(input) {
  function inner(otherInput){
    if(typeof otherInput === 'undefined')
        if(typeof input === 'undefined')
            return f({acc:'foo'});
        else
            return f({acc:input.acc + 'o'});

    if(typeof input !== 'undefined')
        return input.acc + otherInput;
    else
        return 'fo' + otherInput;
  };
  if(typeof input === 'string')
    return "f" + input;
  return inner;
}

How can this be improved?

Review on code.
The term "ugly" is subjective. For some ugly is bad practice or anti-patterns, and there are others that will argue that current standards of good practice are "ugly" due to inefficiencies.
I will take ugly in this case to mean inelegant in the mathematical sense of the term. Thus below is a review in terms of improving code for simplicity and efficiency.
Simplify.
There is no reason to use statement clause for undefined in the form typeof blah === "undefined". Assuming that no input will equate to false apart from no arguments then if(! input) { is simpler.
However, there may be an input like f()(false), where the expected output is "fofalse", which will require a more robust test for undefined. So the safe option without knowledge of possible input is if(input === undefined){ which is still simplier (and more efficient).
For each call of the function f there are only two possible return values, the string that terminates the chain of calls or the function that is used to expand the return string.
Your inner function has 4 return possibilities, which is an obvious unneeded complication.
Simplicity also means not to repeat code. The outer function and inner function have the same functionality. Thus the outer function only needs to begin a process of building a string. This can be done via a closure, so all the outer function need do is define a communicating variable and a function to hold the closure. It does not need to do any form of logic.
Efficiency
Simplicity and efficiency go hand in hand.
The inner function should only return a string or a function to do the same.
The communication between the function calls is where you have added too much overhead with the creation of objects to communicate to the outer function.
You can use a closure variable to communicate information between calls and reduce the call stack depth by not needing to call the outer function. The closure is maintained by returning the inner function in the case of an undefined input.
The result.
The result is hopefully less ugly in your eyes. For me it is far simpler and nicely elegant.

function f(str) {
    var content = "f";
    return (function inner(str){
        if (str !== undefined) { return content + str }
        content += "o";
        return inner;
    })(str);
}

console.log(f("a"));
console.log(f()("a"));
console.log(f()()("a"));

Alternative.
I did toy with the following version but for me it is not as elegant due to the forced flow unique to Javascript that will put those from other languages out of joint. Though it is simpler in terms of token count but not in complexity.

function f(str) {
    var content = "f";
    return (function inner(str){
        return str ? content + str : ( content += "o", inner );
    })(str);
}

console.log(f("a"));
console.log(f()("a"));
console.log(f()()("a"));


Sorting array with respect to two properties I'm sorting with respect to Width and Length. However, Width has the significance precedence, so Length is only vital if Width is exactly equal (both are integers). The comparison method I've implemented as shown below. Is this a neat trick with doubling the most significant value's comparer or is there a better way?

  List<Thing> things = thingsDictionary.Select(thing
    => thing.Value).ToList();
  things.Sort((first, second)
    => 2 * first.Width.CompareTo(second.Width)
       + first.Length.CompareTo(second.Length));

It looks like your implementation relies on Width.CompareTo(second.Width) to return either 1, 0, or -1.

This assumption is wrong. If first.Length is greater than second.Length, CompareTo is allowed to return 9999, or 1, or 500. These could very well screw up your additions.

Sure, the current .NET implementation returns either 1, 0, or -1, but that's an implementation detail. Furthermore, other C# implementations or even future version of .NET may return other values.

Also, tricks reduce readability.

A correct implementation would be:

things.Sort((first, second)
    => {
        var widthComparison = first.Width.CompareTo(second.Width);
        if(widthComparison == 0)
            return widthComparison;
        else return first.Length.CompareTo(second.Length);
    });

But instead of fixing your implementation, I would suggest using LINQ's OrderBy and ThenBy instead. It doesn't seem like you need to sort the list in-place, so there's no need to reinvent the wheel.

List<Thing> things = thingsDictionary.Select(thing => thing.Value)
                                     .OrderBy(x => x.Width)
                                     .ThenBy(x => x.Length)
                                     .ToList();


Union and intersection of two arrays Problem:

  Write an algorithm, to find the union and intersection sorted in ascending order, between the elements of two arrays

  Input Format

  The first line contains an integer, n1 , denoting the size of the first array.

  The second line contains n1 space-separated integers representing elements of the first array.

  The third line contains an integer, n2 , denoting the size of the second array.

  The fourth line contains n2 space-separated integers representing elements of the second array.

  Constraints

  Apart from Array, no other data structure can be used
      Output elements should be sorted in ascending order
      Array should be sorted using Bubble Sort

  Output Format

  First line, union of space-separated elements of both array elements, sorted in ascending order

  Second line, intersection of space-separated elements of both arrays elements, sorted in ascending order

Solution:

"""
   1) Sort two arrays
   2) Find union and intersecton of two sorted arrays
"""
def swap(array, indexI, indexJ):
    temp = array[indexI]
    array[indexI] = array[indexJ]
    array[indexJ] = temp

def bubbleSort(array):
    n = len(array)
    for indexI in range(n):
        exchanges = 0
        for indexJ in range(n-1, indexI, -1):
            if array[indexJ] < array[indexJ-1]:
                swap(array, indexJ, indexJ-1)
                exchanges += 1
        if exchanges == 0:
            break
def intersection(arrayOne, arrayTwo):
    arrayOneIndex = 0
    arrayTwoIndex =0
    sizeOfArrayOne = len(arrayOne)
    sizeOfArrayTwo = len(arrayTwo)
    array = []
    while arrayOneIndex < sizeOfArrayOne and arrayTwoIndex < sizeOfArrayTwo:
        if arrayOne[arrayOneIndex] < arrayTwo[arrayTwoIndex]:
            arrayOneIndex += 1
        elif arrayOne[arrayOneIndex] > arrayTwo[arrayTwoIndex]:
            arrayTwoIndex += 1
        else:
            array.append(arrayOne[arrayOneIndex])
            arrayOneIndex += 1
            arrayTwoIndex += 1
    return array

def union(arrayOne, arrayTwo):
    arrayOneIndex = 0
    arrayTwoIndex =0
    sizeOfArrayOne = len(arrayOne)
    sizeOfArrayTwo = len(arrayTwo)
    array = []
    while arrayOneIndex < sizeOfArrayOne and arrayTwoIndex < sizeOfArrayTwo:
        if arrayOne[arrayOneIndex] < arrayTwo[arrayTwoIndex]:
            array.append(arrayOne[arrayOneIndex])
            arrayOneIndex += 1
        elif arrayOne[arrayOneIndex] > arrayTwo[arrayTwoIndex]:
            array.append(arrayTwo[arrayTwoIndex])
            arrayTwoIndex += 1
        else:
            array.append(arrayOne[arrayOneIndex])
            arrayOneIndex += 1
            arrayTwoIndex += 1
    if arrayOneIndex == sizeOfArrayOne:
        while arrayTwoIndex < sizeOfArrayTwo:
            array.append(arrayTwo[arrayTwoIndex])
            arrayTwoIndex += 1
    else:
        while arrayOneIndex < sizeOfArrayOne:
            array.append(arrayOne[arrayOneIndex])
            arrayOneIndex += 1
    return array

if __name__ == '__main__':
    sizeOne = int(input())
    try:
        arrayOne = [int(x) for x in input().split()]
    except:
        print('Invalid input')
        sys.exit()
    sizeTwo = int(input())
    try:
        arrayTwo = [int(x) for x in input().split()]
    except:
        print('Invalid syntax')
        sys.exit()
    bubbleSort(arrayOne)
    bubbleSort(arrayTwo)
    intersectionArray = intersection(arrayOne, arrayTwo)
    unionArray = union(arrayOne, arrayTwo)
    print(*unionArray, sep=" ")
    print(*intersectionArray, sep=" ")

Correctness

Input

3
4 5 7
5
6 9 5 7 1

Output

1 4 5 6 7 9
5 7

Input

6
1 1 8 9 0 5
4
0 7 3 4

Output

0 1 3 4 5 7 8 9
0

Question:

0) How to avoid such syntax? while arrayOneIndex < sizeOfArrayOne and arrayTwoIndex < sizeOfArrayTwo

1) Does the code require better error handling?

2) Can this code get more pythonic?

Advice 1

PEP 8 has some (minor) complains: the names of functions and their arguments should be funky_functions instead of funkyFunctions, and funky_argument instead of funkyArgument.

Advice 2

In Python, you can say

def funky1():
    for i in range(10):
        yield i

def funky2():
    arr = []
    for i in range(10):
        arr.append(i)
    return arr

So you may come up with:

def intersection(array_one, array_two):
    array_one_index = 0
    array_two_index = 0
    size_of_array_one = len(array_one)
    size_of_array_two = len(array_two)
    while array_one_index < size_of_array_one and array_two_index < size_of_array_two:
        if array_one[array_one_index] < array_two[array_two_index]:
            array_one_index += 1
        elif array_one[array_one_index] > array_two[array_two_index]:
            array_two_index += 1
        else:
            yield array_one[array_one_index]
            array_one_index += 1
            array_two_index += 1

Advice 3

Your union does not omit duplicates. You can deal with them in a following manner:

def union(array_one, array_two):
    array_one_index = 0
    array_two_index = 0
    size_of_array_one = len(array_one)
    size_of_array_two = len(array_two)
    while array_one_index < size_of_array_one and array_two_index < size_of_array_two:
        if array_one_index > 0 and array_one[array_one_index - 1] == array_one[array_one_index]:
            array_one_index += 1
            continue
        if array_two_index > 0 and array_two[array_two_index - 1] == array_two[array_two_index]:
            array_two_index += 1
            continue
        if array_one[array_one_index] < array_two[array_two_index]:
            yield array_one[array_one_index]
            array_one_index += 1
        elif array_one[array_one_index] > array_two[array_two_index]:
            yield array_two[array_two_index]
            array_two_index += 1
        else:
            yield array_one[array_one_index]
            array_one_index += 1
            array_two_index += 1

    while array_two_index < size_of_array_two:
        if array_two_index > 0 and array_two[array_two_index - 1] == array_two[array_two_index]:
            array_two_index += 1
            continue
        yield array_two[array_two_index]
        array_two_index += 1

    while array_one_index < size_of_array_one:
        if array_one_index > 0 and array_one[array_one_index - 1] == array_one[array_one_index]:
            array_one_index += 1
            continue
        yield array_one[array_one_index]
        array_one_index += 1

Advice 4

Note also that you don't need

if arrayOneIndex == sizeOfArrayOne:

in union; only one of the "leftover" loop will iterate at least once.

Hope that helps.


Eratosthenes' and Sundaram's prime sieves in Python I recently started playing with Python. What can I do better?

def sieve_of_sundaram(limit):
    new_limit = (limit - 1) // 2
    boolean_list = [True] * new_limit;
    i = 1
    j = i
    while (i + j + 2 * i * j <= new_limit):
        while (i + j + 2 * i * j <= new_limit):
            boolean_list[2 * i * j + i + j - 1] = False
            j += 1
        i += 1
        j = i
    primes = [2]
    for i in range(0, new_limit):
        if boolean_list[i]:
            primes.append(i * 2 + 3)
    return primes

def sieve_of_eratosthenes(limit):
    boolean_list = [True] * limit;
    for i in range(2, limit):
        if i * i > limit:
            break
        while boolean_list[i - 1] == False:
            i += 1
        for j in range(i * i, limit + 1, i):
            boolean_list[j - 1] = False
    primes = []
    for i in range(1, limit):
        if boolean_list[i] == True:
            primes.append(i + 1)
    return primes

i = 100
sundaram_primes = sieve_of_sundaram(i)
eratosthenes_primes = sieve_of_eratosthenes(i)

print("Sundaram primes count: ", len(sundaram_primes))
print(sundaram_primes)
print("Eratosthenes primes count: ", len(eratosthenes_primes))
print(eratosthenes_primes)

There are some curious inconsistencies between the Eratosthenes and Sundaram code.

    for i in range(2, limit):
        if i * i > limit:
            break

is IMO more Pythonic than

    i = 1
    j = i
    while (i + j + 2 * i * j <= new_limit):
        ...
        i += 1
        j = i

We can refactor the second into something similar to the first: firstly, observe that there's no need to repeat the assignment j = i:

    i = 1
    while (i + i + 2 * i * i <= new_limit):
        j = i
        ...
        i += 1

Then simplify algebraically:

    i = 1
    while (2 * i * (i + 1) <= new_limit):
        j = i
        ...
        i += 1

And finally replace with range:

    for i in range(1, new_limit + 1):
        if 2 * i * (i + 1) > new_limit:
            break
        j = i
        ...

As noted by Janne, you can then refactor the inner loop to also use range...

The indexing of the arrays in both is a bit tricky. Are the maintenance costs worth saving four bytes of memory? If so, would it at least be worth leaving a comment to explain why (for example) the primes recovered by the Sundaram sieve are 2 * i + 3 instead of the 2 * i + 1 which someone who just read the Wikipedia page would expect?

    primes = [2]
    for i in range(0, new_limit):
        if boolean_list[i]:
            primes.append(i * 2 + 3)

can be written as a one-liner using something called list comprehensions. This is something that's worth learning because it's very common style for Python.

    primes = [2] + [i * 2 + 3 for i in range(0, new_limit) if boolean_list[i]]

        if boolean_list[i]:

...

        if boolean_list[i] == True:

What's the difference?

Answer: the first one is better style. x == True should be rewritten as x ; and y == False or y != True should be rewritten as not y. With good variable names this usually gives a very natural reading.

On the subject of names: boolean_list tells me the type, but what I care about is the meaning. For Eratosthenes that's easy: a True means that the index is a prime, and a False means that it's a composite number, so is_prime is a good name. For Sundaram the exact interpretation is a bit trickier, but is_prime would still work.


Generic Type Trait example I want to check that a some type has some methods:

using Arg_t2 = int;
using Arg_t3 = int;

template<class T>
struct ValidType{
   using Ret_t1 = T;
   using Arg_t1 = int;
   // ...
   Ret_t1 method1(Arg_t1,Arg_t2,Arg_t3);
   //...
};

Therefor I wrote a type-trait:

#include <type_traits>

template<class T,class _ = void> struct is_my_valid : ::std::false_type {};
template<class T> struct is_my_valid<T,::std::enable_if_t<
  ::std::is_same<typename T::Ret_t1,decltype(::std::declval<T>().method1(::std::declval<typename T::Arg_t1>(),::std::declval<Arg_t2>(),::std::declval<Arg_t3>()))>::value
  // && ...
> > : ::std::true_type {};

We can test this with:

static_assert(is_my_valid<ValidType<int>>::value,"");

What are my options to make the definition of is_my_valid this look prettier?

Edit:
I basicly want to describe an Interface with type traits. I want to do this, because the interface includes types, that depend on template parameters.

I also like that an type does not have to be implemented using the ValidType in anyway to be valid.

Edit: Only after posting did I see that the question was tagged c++14. This code relies on std::void_t from c++17 which is trivial to implement in c++14.

You can extract some of the functionality to a reusable detect type-trait that only needs to be supplied a type alias to work.

namespace detail {

template <template <class...> class CheckTemplate, class = void, class... Object>
struct detect_impl : std::false_type {};

template <template <class...> class CheckTemplate, class... Object>
struct detect_impl<CheckTemplate, std::void_t<CheckTemplate<Object...>>, Object...> : std::true_type {};
}  // namespace detail

template <template <class...> class CheckTemplate, class... Object>
constexpr bool detect_v = detail::detect_impl<CheckTemplate, void, Object...>{};

With that in place all we need is to use it as such.

template <class T>
using is_valid = decltype(std::declval<T>().method1(0, 0, 0));

template <typename T>
constexpr bool is_valid_v = detect_v<is_valid, T>;

static_assert(is_valid_v<ValidType<int>>)

The Object pack let's us make a more flexible is_valid that takes the argument types as parameters if we want.

template <class T, class... Args>
using is_valid_flex = decltype(std::declval<T>().method1(std::declval<Args>()...));

template <class... Args>
constexpr bool is_valid_flex_v = detect_v<is_valid, Args...>;

static_assert(is_valid_flex_v<ValidType<int>, int, int int>);

Additional modification is possible if we want to involve the return-type of the method.


Swift - Finding longest binary gap of a given integer I recently had the chance to try Codility's algorithm training, and the very first one in the list is finding the longest binary gap of a given integer.
I tried to implement this in Swift, and after finishing it up, I looked at answers all over the internet. My approach is a bit different from what I have seen from others.

What is a Binary Gap?

A binary gap within a positive integer N is any maximal sequence of consecutive zeros that is surrounded by ones at both ends in the binary representation of N.
For example, number 9 has binary representation 1001 and contains a binary gap of length 2. The number 529 has binary representation 1000010001 and contains two binary gaps: one of length 4 and one of length 3. The number 20 has binary representation 10100 and contains one binary gap of length 1. The number 15 has binary representation 1111 and has no binary gaps. The number 32 has binary representation 100000 and has no binary gaps.

The following is from the console which makes it easy to understand the implementation.
Binary String of "1041" is: "10000010001"
Position of 1's: [0, 6, 10]
The longest binary gap is 5

Basically what I am doing is:

Iterating each binary character, and store the position/index of 1's in an array.
As I am looping through, I find the binary gap between two 1's by subtracting the current index of 1 with previous index of 1, so that it gives me how many 0's are present in between them. I then decrement whatever value I get by 1 ('cause arrays...).
As I am looping through, I keep track of the longest binary gap in a variable.

The complexity is O(n), and I'd like to know if this approach is in any way efficient in terms of memory, or any scope of improvements with this? Thanks for your time.

Code:
public func getLongestBinaryGapFor(_ N : Int) -> Int {

    var arrayOfIndexes:[Int] = []
    let binaryString = String(N, radix:2)

    print("Binary String of \"\(N)\" is: \"\(binaryString)\"")

    var longestBinaryGap:Int = 0
    var index = 0

    for char in binaryString {
        if char == "1" {
            arrayOfIndexes.append(index)
            let currentBinaryGap = getCurrentBinaryGapFor(arrayOfIndexes)
            if arrayOfIndexes.count == 2 {
                longestBinaryGap = currentBinaryGap
            } else if index > 2 {
                if currentBinaryGap > longestBinaryGap {
                    longestBinaryGap = currentBinaryGap
                }
            }
        }
        index += 1
    }

    print("Position of 1's: \(arrayOfIndexes)")
    return longestBinaryGap
}

func getCurrentBinaryGapFor(_ array:[Int]) -> Int {
    var currentBinaryGap = 0
    if array.count >= 2 {
        let currentPosition = array.count - 1
        let previousPosition = currentPosition - 1
        currentBinaryGap = array[currentPosition] - array[previousPosition] - 1
        return currentBinaryGap
    } else {
        return currentBinaryGap
    }
}

Naming

According to the
API Design Guidelines,
needless word should be omitted, and  prepositional phrases should be made
argument labels. Also variable names should generally be lower (camel) case.

Therefore a better function name would be

public func longestBinaryGap(for n: Int) -> Int

Also variable names should not contain type information, that applies
for example to

var arrayOfIndexes: [Int]

Simplifying your approach

Let's start with func getCurrentBinaryGapFor(), that looks unnecessary
complicated. Without additional local variables it can be written as

func getCurrentBinaryGapFor(_ array:[Int]) -> Int {
    if array.count >= 2 {
        return array[array.count - 1] - array[array.count - 2] - 1
    } else {
        return 0
    }
}

In the main function,

let currentBinaryGap = getCurrentBinaryGapFor(arrayOfIndexes)
if arrayOfIndexes.count == 2 {
    longestBinaryGap = currentBinaryGap
} else if index > 2 {
    if currentBinaryGap > longestBinaryGap {
        longestBinaryGap = currentBinaryGap
    }
}

can be simplified to

let currentBinaryGap = getCurrentBinaryGapFor(arrayOfIndexes)
if currentBinaryGap > longestBinaryGap {
    longestBinaryGap = currentBinaryGap
}

and the loop

var index = 0
for char in binaryString {
    if char == "1" {
        // ... do something ...
    }
    index += 1
}

can be written as

for (index, char) in binaryString.enumerated() where char == "1" {
    // ... do something ...
}

Only the last two elements of arrayOfIndexes are used in the program.
I would

either store all positions of ones in the array first, and then determine
the longest run from that array once,
or use two variables for the recent two positions instead of an array.

In either case, a separate function to compute the current gap seems
unnecessary.

Summarizing the changes so far, the function would look like this:

public func longestBinaryGap(for n: Int) -> Int {
    var bitPositions: [Int] = [] // Positions of all `1` bits in `n`
    let binaryRepr = String(n, radix:2) //

    for (index, char) in binaryRepr.enumerated() where char == "1" {
        bitPositions.append(index)
    }

    var longestGap: Int = 0
    if bitPositions.count >= 2 {
        for i in 0...bitPositions.count - 2 {
            let gap = bitPositions[i + 1] - bitPositions[i] - 1
            if gap > longestGap {
                longestGap = gap
            }
        }
    }

    return longestGap
}

Using Swift's functional methods this could be written as

public func longestBinaryGap(for n: Int) -> Int {
    let binaryRepr = String(n, radix: 2) //

    let bitPositions = binaryRepr.enumerated()
        .filter { $0.element == "1" }
        .map { $0.offset }

    let longestGap = zip(bitPositions, bitPositions.dropFirst())
        .map { $1 - $0 - 1 }
        .max() ?? 0

    return longestGap
}

which looks elegant, but is actually a bit slower than the previous version.

Boosting the performance

Your approach is \$ O(d) \$ (in time and memory), where \$ d \$ is the
number of digits in \$ n \$. That seems very good since any solution must
somehow traverse all digits of the given number.

As said above, the memory usage can be reduced to \$ O(1) \$ by keeping
only the last two bit positions, instead of using an array.

However, much time can be saved by not converting the integer to a string.
The binary digits of a number can be determined efficiently in pure
integer arithmetic.

That leads to the following implementation (essentially a Swift version
of Find the binary gap of a number N),
which is considerably faster:

public func longestBinaryGap1(for n : Int) -> Int {

    if n <= 0 {
        return 0
    }

    var n = n // Make a mutable copy
    // Remove trailing zeros:
    while n % 2 == 0 {
        n /= 2
    }

    var longestGap = 0
    var gap = 0
    repeat {
        n /= 2
        if n % 2 == 0 {
            // Current bit is `0`: increase gap size.
            gap += 1
        } else {
            // Current bit is `1`: check current gap, then reset.
            if gap > longestGap {
                longestGap = gap
            }
            gap = 0
        }
    } while n > 0

    return longestGap
}

And it gets even faster if we use the “find first bit set”
function ffsl() (which might take advantage of compiler intrinsics):

public func longestBinaryGap2(for n : Int) -> Int {

    var n = n // Make a mutable copy
    let firstPosition = ffsl(n)
    if firstPosition == 0 {
        return 0
    }
    n = n >> firstPosition

    var longestGap = 0
    while n > 0 {
        let position = Int(ffsl(n))
        if position - 1 > longestGap {
            longestGap = position - 1
        }
        n = n >> position
    }

    return longestGap
}

Benchmark

My simple benchmark:

let M = 1_000_000

do {
    var sum = 0
    let start = Date()
    for n in 1...M { sum += longestBinaryGap(for: n) }
    let end = Date()
    print(sum, end.timeIntervalSince(start))
}

On a 1.2 GHz Intel Core m5 (with the program compiled in Release mode, i.e.
with optimizations), I measured approximately:

Your original code: 3.3 seconds
longestBinaryGap1(): 0.08 seconds
longestBinaryGap2(): 0.04 seconds


Shape area calculation I know questions with this text has been asked and already answered, but my question is mainly limited to only one method of this flow and where it should be.

I have an assembly for Shape's area calculation (ShapeAreaCalculator) and this functionality is exposed to client program:

public enum ShapeType
{
    SquareShape,
    CircleShape,
    TriangleShape,
}
public interface IShape
{
    void CalculateArea();
    void ShowArea();
}
public class Square : IShape
{
    public void ShowArea()
    {
        Console.WriteLine("Logic For ShowArea of Square");
    }
    public void CalculateArea()
    {
        Console.WriteLine("Logic For CalcualteArea of Square");
    }
}
public class Circle : IShape
{
    public void ShowArea()
    {
        Console.WriteLine("Logic For ShowArea of Circle");
    }
    public void CalculateArea()
    {
        Console.WriteLine("Logic For CalculateArea of Circle");
    }
}
public class Triangle : IShape
{
    public void ShowArea()
    {
        Console.WriteLine("Logic For ShowArea of Triangle");
    }
    public void CalculateArea()
    {
        Console.WriteLine("Logic For CalculateArea of Triangle");
    }
}

Factory class:

public interface IShapeFactory
{
    IShape GetShape(ShapeType shapeType);
}
public class ShapeFactory : IShapeFactory
{
    public IShape GetShape(ShapeType shapeType)
    {
        switch(shapeType)
        {
            case ShapeType.SquareShape:
                return new Square();
            case ShapeType.CircleShape:
                return new Circle();
            case ShapeType.TriangleShape:
                return new Triangle();
            default :
                throw new ArgumentException();

        }
    }
}

And I'm using this at client side like this:

static void Main(string[] args)
{
    ShapeType selectedShape;
    while (true)
    {
        Console.WriteLine("Enter your choise");
        foreach (ShapeType shapeType in Enum.GetValues(typeof(ShapeType)))
        {
            Console.WriteLine((int)shapeType + " For " + Enum.GetName(shapeType.GetType(), shapeType));
        }
        int selection;
        if (int.TryParse(Console.ReadLine(), out selection) && Enum.IsDefined(typeof(ShapeType), selection))
        {
            selectedShape = (ShapeType)selection;
            break;
        }
    }
    CalculateAndShowAreaOfShape(selectedShape);
}
static void CalculateAndShowAreaOfShape(ShapeType selectedShape)
{
    IShapeFactory shapeFactory = new ShapeFactory();
    IShape currentShape = shapeFactory.GetShape(selectedShape);
    currentShape.CalculateArea();
    currentShape.ShowArea();
}

But now I am concerned about this method CalculateAndShowAreaOfShape(). If in the future some functionality added in the procedure of area calculation, client code will be needed to be modified. It is unnecessary coupling.

Should it be implemented at client end or somewhere in ShapeAreaCalculator assembly?

Also, is it good practice to ask for user input from assembly? (I think it will limit this assembly to be used in console application only.)

I think your names could be simplified.  For instance, the ShapeType enum could be:

public enum ShapeType { Square, Circle, Triangle }

Because ShapeType.SquareShape sounds redundant, whereas ShapeType.Square still clearly indicates that the square is a shape type.

The ShowArea method is not needed.  It would most likely be used to display console writes, but your shape classes should not concern themselves with writing to the console.  You could modify ShowArea to return a string, but that string would be nothing more than the numeric area converted to a string.

CalculateAndShowArea should be in the client, as it issues several calls and it - not the respective IShape instance - should take care of console writes.

But most importantly, the area calculation should be done within the shape classes.  The problem with your code as posted is it lacks many critical things that help distinguish among different shape types.  For instance, a circle would be defined at the very least by a radius, and you may consider a center XY point along with a radius.  This allows you to calculate the circle's area.  Likewise, a square should be defined by a length of one of its sides.  And again, a square could then calculate its area.

A triangle is a different creature.  There are different ways to construct a triangle.  One way is passing in 3 non-colinear points.  Another way might be passing in 3 line segment lengths.  But a triangle constructor would also have to contain validation that whatever was passed in creates a truly valid triangle.

Which makes a singular factory method to generate 3 very different shapes fairly useless, unless you imposed a restriction that it would be a circle with radius 1, a square with side length of 1, and a triangle with 2 sides of length 1.  But if you do that, you've reduced the flexibility of the specific shape classes.

Also, the method name CalculateArea is bit long.  A more standard name would be GetArea, but many people would really choose to have a readonly property named Area.

Example

public interface IShape
{
    double Area { get; }
    string Name { get; }
}
public class Square : IShape
{
    // May consider Length be a double
    public int Length { get; }
    public Square(int side)
    {
        Length = side;
    }
    public double Area => Length * Length;
    public string Name => "Square";
    public override string ToString() => $"{Name}: Length={Length}";
}
public class Circle : IShape
{
    // Might consider making this a double
    public int Radius { get; }
    public Circle(int radius)
    {
        Radius = radius;
    }
    public double Area => Math.PI * Radius * Radius;
    public string Name => "Circle";
    public override string ToString() => $"{Name}: Radius={Radius}";
}
// Triangle class is left to you.

This makes your shape constructor more tailor made for each specific shape, but it also means you may not be able to construct it via a singular factory method without some significant changes.

UPDATE

As noted by Jesse Slicer in the comments, each class could identify itself better.  Jesse suggests having each class could have a Shape property that would echo its ShapeType.  It a decent idea but one that I would modify slightly.  I see the ShapeType enum as belong to the factory method.  I think a design should allow for the possibility of a shape that isn't specified in the enum but that implements IShape.  Thus I would have each class implement its type, and we can ditch the Name property.

public interface IShape
{
    double Area { get; }
    Type Type { get; }
}
public class Square : IShape
{
    // May consider Length be a double
    public int Length { get; }
    public Square(int side)
    {
        Length = side;
    }
    public double Area => Length * Length;
    public Type Type => GetType();
    public override string ToString() => $"{Type.Name}: Length={Length}";
}
public class Circle : IShape
{
    // Might consider making this a double
    public int Radius { get; }
    public Circle(int radius)
    {
        Radius = radius;
    }
    public double Area => Radius * 2 * Math.PI;
    public Type Type => GetType();
    public override string ToString() => $"{Type.Name}: Radius={Radius}";
}


Summing 3 consecutive integers in a list to see if the sum matches a specified value I have a program where users can add integers to a list and then I want to check the list to see if any three consecutive integers in the list sum to a specified value. I have been told that it is taking too long when ran with a large number of integers in the list. The timing test that I am not passing is based upon looking for the first match of the sum of 3 consecutive integers, in a large list of integers, to one specific value. The specific value can vary with each run but not within the run and the single run is that is being timed.

class Aggro
{
    private List<int> _numbers = new List<int>();
    public void AddToList(int[] list)
    {
        for (int i = 0; i < list.Length; i++ )
        {
            _numbers.Add(list[i]);
        }
    }

    public bool SumExists(int sum)
    {
        int[] numbers = _numbers.ToArray();
        for (int i = 0; i < numbers.Length -2; i++)
       {
            if (numbers[i] + numbers[i + 1] + numbers[i + 2] == sum)
            {
                return true;
            }
        }
        return false;
    }

    public static void Main(string[] args)
    {
        Aggro a = new Aggro();
        a.AddToList(new int[] { 12, 21, 16, 4, 67, 1 });
        Console.WriteLine(a.SumExists(41)); // True
        Console.WriteLine(s.SumExists(69)); // False
        a.AddToList(new int[] { 1 });
        Console.WriteLine(a.ContainsSum3(69)); //True
    }
}

About performance:

One call of a.AddToList plus one call of a.SumExists cannot work conceptually faster than your variant. But several calls of a.AddToList plus several calls of a.SumExists can run much faster.

You can index all the sums after first call to a.SumExists with as HashSet, so all other calls of SumExists will be run in \$O(1)\$.
A second (and others) call to a.AddToList needs no recalc the whole index - you need only add new elements.

class Aggro
{
    List<int> _numbers = new List<int>();
    HashSet<int> _index = new HashSet<int>();
    int _lastIndexed = -1;

    public void AddToList(int[] list)
    {
        for (int i = 0; i < list.Length; i++ )
        {
            _numbers.Add(list[i]);
        }
    }

    public bool SumExists(int sum)
    {
       if (_lastIndexed > -1 && _index.Contains(sum)) return true;
       for (int i = _lastIndexed + 1, c = numbers.Length - 2; i < c; i++)
       {
            int tmp = numbers[i] + numbers[i + 1] + numbers[i + 2];
            _index.Add(tmp);
            _lastIndexed = i;
            if (tmp == sum)  return true;
        }
        return false;
    }

    public static void Main(string[] args)
    {
        Aggro a = new Aggro();
        a.AddToList(new int[] { 12, 21, 16, 4, 67, 1 });
        Console.WriteLine(a.SumExists(41)); // True
        Console.WriteLine(s.SumExists(69)); // False
        a.AddToList(new int[] { 1 });
        Console.WriteLine(a.ContainsSum3(69)); //True
    }
}


Simulate Tic-Tac-Toe game in Python Simulate a tic-tac-toe game

Requirements

A player is picked at random as a first player.
The two players plays tic tac toe game.
The game ends when the board is full or either one of the player wins.
Return the board status and result after end of the game.

  Player Class

class Player:
    def __init__(self, name, symbol):
        self.name = name
        self.symbol = symbol

    def pick_available_grid(self, available_position):
        random_index = random.choice(available_position.keys())
        return random_index

  Board Class

class Board:
    def __init__(self):
        self.grid = [None]*9
        self.available_position = {1: 0, 2: 0, 3: 0, 4: 0, 5: 0, 6: 0, 7: 0, 8: 0, 9: 0}

    def update_board(self, index, symbol):
        self.grid[index-1] = symbol
        del self.available_position[index]

    def check_if_board_full(self):
        len(self.available_position) == 0

    def check_for_win_condition(self,symbol):
        if (self.grid[0]==symbol and self.grid[1]==symbol and self.grid[2]==symbol):
            return True
        if (self.grid[3]==symbol and self.grid[4]==symbol and self.grid[5]==symbol):
            return True
        if (self.grid[6]==symbol and self.grid[7]==symbol and self.grid[8]==symbol):
            return True

        #vertical grid check
        if (self.grid[0]==symbol and self.grid[3]==symbol and self.grid[6]==symbol):
            return True
        if (self.grid[1]==symbol and self.grid[4]==symbol and self.grid[7]==symbol):
            return True
        if (self.grid[2]==symbol and self.grid[5]==symbol and self.grid[8]==symbol):
            return True

        #diagonal grid check
        if (self.grid[0]==symbol and self.grid[4]==symbol and self.grid[8]==symbol):
            return True
        if (self.grid[2]==symbol and self.grid[4]==symbol and self.grid[6]==symbol):
            return True

  Game Class

class Game:
    global SYMBOL
    SYMBOL = ['X', 'O']

    def __init__(self):
        self.board = Board()
        self.players = []
        self.result = None

    def set_players(self):
        player1 = Player('akanksha', SYMBOL[0])
        player2 = Player('akanksha', SYMBOL[1])
        self.players.append(player1)
        self.players.append(player2)

    def set_current_player(self, current_player=None):
        current_player = self.players[1] if current_player == self.players[0] else self.players[0]
        return current_player

    def set_result(self):
        if self.board.check_for_win_condition(self.players[0].symbol):
            self.result = self.players[0]
        elif self.board.check_for_win_condition(self.players[1].symbol):
            self.result = self.players[1]
        elif self.board.check_if_board_full():
            self.result = 'Draw'
        return self.result

    def play_game(self):
        current_player = self.set_current_player()
        while(not self.set_result()):
            available_position = self.board.available_position
            grid_position = current_player.pick_available_grid(available_position)
            self.board.update_board(grid_position, current_player.symbol)
            current_player = self.set_current_player(current_player)

    def end_game(self):
        print(self.board.grid)
        if self.result == 'Draw':
            print('This game is a draw')
        else:
            print('player', self.result.name, "with symbol", self.result.symbol, "wins")

    def start_game(self):
        self.set_players()
        self.play_game()
        self.set_result()
        self.end_game()

  Main Program

class MainProgram:
    def main():
        print("Welcome to TicTacToe Game")
        game = Game()
        game.start_game()

    main()

I am new to oops, how can I improve this?

Questions:

Should I create a separate class for symbol?
Should check_for_win_condition be included in Game instead of Board class?
Should self.available_position be used or it should be method which computes available cells from grid

Honestly, for the symbol, I'd just go with using uppercase characters (X and O) since the pieces don't really have any special functionality at this stage, and it's good to keep some simplicity with earlier projects.

I'd put the win condition checking into the game class. If you want to reuse the board class in the future, it's a good idea for you to avoid having any implementation-specific code (i.e. functionality that's specific to Tic-Tac-Toe) in the class - when you're dealing with Object Orientied Programming, you have to consider SOLID principles.

In terms of self.available_positions, I'd forego it entirely. You already have a structure which keeps track of the board, so adding a second structure to keep track of available positions is largely redundant - because one is just the inverse of the other (i.e. self.available_positions are the null elements in self.grid), and duplication is wasting memory and can lead to error (it's also good practice to have a single source of truth).

In terms of code, it's a good start, but I have a few suggested modifications for the Board class:

class Board:
    def __init__(self):
        self.grid = {1: '', 2: '', 3: '', 4: '', 5: '', 6: '', 7: '', 8: '', 9: }
        # Board starts with 9 available positions, and we'll decrement it by one.
        # I've changed to this from the self.available_position dictionary because you were
        # essentially maintaining two structures to maintain the progress of the game,
        # and having a single integer that we decrement is easier.
        self.remaining_turns = 9

    def update_board(self, index, symbol):
        self.grid[index-1] = symbol
        # Decrease the number of available turns after placing a piece.
        self.remaining_turns -= 1
        # Probably worth checking for 0 remaining turns after performing this decrement.

    def check_for_win_condition(self,symbol):
        # This if statement will be entered if any of the functions return true

        if check_diagonal(symbol) or check_horizontal(symbol) or check_vertical(symbol):
            return True

    def check_horizontal(self, symbol):
        # 0,3,6 refer to the starting elements in your vertical grid check, the additions
        # refer to the offsets to get to the other elements in the line
        for element in [0, 3, 6]:
            if self.grid[element]==self.grid[element+1]==self.grid[element+2]==symbol:
                return True

    def check_vertical(self, symbol):
        # 0,1,2 refer to the starting elements in your vertical grid check, the additions
        # refer to the offsets to get to the other elements in the line
        for element in [0, 1, 2]:
            if self.grid[element]==self.grid[element+3]==self.grid[element+6]==symbol:
                return True

    def check_diagonal(self, symbol):
        # You can chain equality checks
        if (self.grid[0]==self.grid[4]==self.grid[8]==symbol) or (self.grid[2]==self.grid[4]==self.grid[6]==symbol):
            return True

It's just something I typed up quickly, and I haven't had the opportunity to test it yet. My biggest issue was with the check_for_win_condition function - you were repeating a lot of the same checks (just altering the element reference for the grid).

In terms of language choice; Python is great - it's versatile, and it's intuitive - however, go with Python 3. Python 2 is end of life in 2020 - which means it will no longer have support from the Python developers/maintainers, but Python 3 is in active development, and it has a number of improvements and new features.


String compression by using repeated characters count My task was to perform a basic string compression by replacing consecutive repeated characters by one instance of the character and integer denoting the number of repetitions.
For example the string "aaaaabbcccdeee" should be reduced to "a5b2c3de3".
I have tried working in C# using a simple logic. Please see the below code. Also I have added the condition to not to add the count 1 in the compressed string if there is only single letter occurrence. E.g. if letter "d" is occurred only once in a string "aaabbcccdee" then function will give a compressed string as "a3b2c3de2" and not the "a3b2c3d1e2". To modify this requirement we can change the condition for this single count.
Please let me know about the comparative time and space complexity and other efficient way in C#.

class StringCompression
{
    static void Main(string[] args)
    {
        StringCompression sc = new StringCompression();

        sc.CompressionMethod("aaaaabbbccdeeeee");
        sc.CompressionMethod("aaabbccdddee");
        sc.CompressionMethod("a");
    }

    public void CompressionMethod(string originalString)
    {
        List<char> OriginalList = new List<char>();
        List<string> CompressedList = new List<string>();
        OriginalList.AddRange(originalString);
        // Convert to Character Array
        char[] charArray = OriginalList.ToArray();

        int i = 0;
        char character;
        int len = charArray.Length;

        while (i < len)
        {
            int n = 0;
            character = (charArray[i]);
            while (i < charArray.Length && charArray[i] == character)
            {
                n = n + 1;
                i++;
            }

            // add characters to the new list
            CompressedList.Add(character.ToString());

            // add character counts to the new list
            if (n == 1)
            {
                // Do nothing
            }
            else
            {
                CompressedList.Add(n.ToString());
            }
        }
        // CompressedList will contain compressed string
        foreach (string str in CompressedList)
        {
            Console.Write(str);
        }
        Console.Write("\n");
        Console.Write("\n");
    }
}

String as an Array

You got your string parameter:

string originalString

Then you turned it into a list:

OriginalList.AddRange(originalString);

Then you turned it into an array:

charArray = OriginalList.ToArray();

And finally used it as:

character = (charArray[i]);

Instead of all that, you could have just done this:

character = (originalString[i]);

Class Design

Instantiating a StringCompression class every time you wanna compress a string can be avoided by just declaring the function CompressionMethod as static since the class has no members anyway. So in Main, you only have to do:

StringCompression.CompressionMethod("String here");

Or better: make it an extension method.

String Builder

Instead of a list of strings in:

CompressedList = new List<string>();

You could instead use a String Builder to create the resulting string.


changing from function based view to class based view in Django This is my attempt to convert my password reset function based view to a class based view. Please suggest improvements.
This is my function based view
from django.contrib.auth.views import password_reset, password_reset_confirm

def reset(request):
    return password_reset(request, template_name='login/reset.html',
        email_template_name='login/reset_email.html',
        subject_template_name='login/reset_subject.txt',
        post_reset_redirect=reverse('success'))

TO change this view to the class based view I have to written this
class ResetPassword(View):
    def get(self, request):
        return password_reset(self.request, template_name='login/reset.html',
            email_template_name='login/reset_email.html',
            subject_template_name='login/reset_subject.txt',
            post_reset_redirect=reverse('success'))
    def post(self, request):
        return password_reset(self.request, template_name='login/reset.html',
        email_template_name='login/reset_email.html',
        subject_template_name='login/reset_subject.txt',
        post_reset_redirect=reverse('success'))

Since both the functions are performing same action, you can just say post = get:

class ResetPassword(View):
    def get(self, request):
        return password_reset(self.request, template_name='login/reset.html',
            email_template_name='login/reset_email.html',
            subject_template_name='login/reset_subject.txt',
            post_reset_redirect=reverse('success')
        )
    post = get


Filter non-duplicate objects from collection I need to filter an array of form controls to check for duplicate values. I have written the following algorithm to identify and return an array of the controls which have are duplicates. I have not seen this indexOf based approach in any of the sample duplicate checkers one finds in Google. Can anyone here spot a flaw in my code?

Testing codepen.

// select all keys
const duplicateKeys = values.map((current) => current.name);
// filter duplicate keys
const filteredDuplicateKeys = duplicateKeys.reduce((prev, name, index, names) => {
  if (names.indexOf(name) !== names.indexOf(name, index)) {
    if (!prev.includes(name)) return prev.concat(name);
  }
  return prev;
}, []);
// filter original array based on identified duplicate keys
const duplicates = values.reduce((prev, current) => filteredDuplicateKeys.indexOf(current.name) > -1 ? prev.concat(current) : prev, []);

The important flaw in your code is runtime complexity.  reduce, indexOf and includes all have runtimes that are linear (O(n)) in the size of the array.  You're running the latter two once for every iteration of reduce, giving a quadratic (O(n2)) runtime.  Doubling the length of the input will quadruple the running time.

A secondary flaw is excess regular human-brain complexity: it's more lines of code and data structures than are needed.

The typical approach involves a single hash table to count duplicates and just two lines of code: one to count, one to filter:

var countKeys = {};
values.forEach( value => countKeys[value.name] = (countKeys[value.name] || 0) + 1 )
const duplicates = values.filter( value => countKeys[value.name]>1 );

const values = [
  {
    name: 'Duplicate 1',
    value: [1,2,3],
  },
  {
    name: 'Duplicate 2',
    value: [1,2,3],
  },
  {
    name: 'Duplicate 1',
    value: [1,2,34],
  },
  {
    name: 'Duplicate 2',
    value: [1,2,3],
  },
  {
    name: 'Duplicate 2',
    value: [1,2,3],
  },
  {
    name: 'Not Duplicate',
    value: [1,2,3],
  },
];

var countKeys = {};
values.forEach( value => countKeys[value.name] = (countKeys[value.name] || 0) + 1 )
const duplicates = values.filter( value => countKeys[value.name]>1 );
console.log(duplicates)
<p>


Javascript obj assignment/mutation in for-loops I have a working function achieving what I want to do, but, I feel that there is a better, concise, and appropriate way to achieve this but I can't quite put my finger on how.

Given an object with (n) recipients, duplicate the message so that there is only ever one recipient and every recipient receives a unique message-id.

here is how I achieved it:

const uuidGenerator = () => Math.random();

const message = {
  message: 'hello',
  recipients: ['John', 'Susie', 'Christian'],
}

function split(obj) {
  const messages = [];
  const recipients = obj.recipients;
  let copy = obj;

  for (let i = 0; i < recipients.length; i++) {
    copy.id = uuidGenerator(),
    copy.recipient = recipients[i];
    copy.message = obj.message
    messages.push(copy);
    copy = {}
  }
  return messages;
}

const arrOfMessages = split(message)
console.log(arrOfMessages)

precisely I am not a fan of how at the end of each loop I am reassigning an obj. I feel... dirty, and it feels like the bigger an object could be the more expensive it could be... what if the message had Base64 video file within it? copying and reassigning feels like this could become more expensive (n2) in big O notation?

any help and clarification would be well appreciated!

I think this is what you're looking for

const uuidGenerator = () => Math.random();

const message = {
  message: 'hello',
  recipients: ['John', 'Susie', 'Christian'],
};

function split(obj) {
  const { message, recipients, ...remaining } = obj;
  return recipients.map((recipient) => {
    return { id: uuidGenerator(), message, ...remaining, recipient: recipient };
  });
}

const arrOfMessages = split(message);
console.log(arrOfMessages);

if there are too many values that you want to copy from the actual obj, then you can use the spread operator i.e., remaining in this case.

However, I am not sure about how efficient this is. These are possible approaches:
- Object.assign
- Spread Operator
- manually adding keys to base Object
Because, I have seen instances where each of these perform differently based on the sample size.

I remember that last approach is faster if your sample size is huge.


VBA root finder algorithm I did two simple functions in VBA (see below).

When a user calls TirNoPer360 in Excel he must select cashflows and dates (as range), then he will get a rate (through iterations according to the second function TirNoPer360).

Function Opt(Caja, Fecha As Range, tasa As Double)

Dim i As Integer
Dim sum As Double
Dim Initial As Date

Initial = Fecha(1, 1)

For i = 1 To Caja.Rows.Count
    sum = sum + Caja(i) / ((1 + tasa) ^ (Application.WorksheetFunction.Days360(Initial, Fecha(i)) / 360))
Next i

Opt = sum

End Function

======================

Function TirNoPer360(Cash, Dates As Range, rate As Double)
Dim tolerance  As Double
tolerance = 0.00001

For Iteration = 1 To 100
    If Opt(Cash, Dates, rate) > 0 And Opt(Cash, Dates, rate) > tolerance Then
        rate = rate + 0.0000001
    ElseIf Opt(Cash, Dates, rate) < 0 And Opt(Cash, Dates, rate) < tolerance * -1 Then
        rate = rate - 0.0000001
    Else
        rate = rate
    End If
    TirNoPer360 = rate
Next Iteration

End Function

When cashflows and dates increase I need more than 100 tries in order to get a good rate (approximated to four decimals) and time increases a lot. So, I would like to speed up my code.
I think I must improve the root finder algorithm, but I don't know how exactly I can do it.
Can anyone help me?

PD: I read Excel uses Newton's algorithm to find the root for a function.

Function TirNoPer360(Cash, Dates As Range, rate As Double)

The function is implicitly Public, implicitly returns a Variant, and implicitly receives all its parameters ByRef; Cash is implicitly a Variant, and the name isn't making it obvious that...

If Opt(Cash, Dates, rate)

...

Function Opt(Caja, Fecha As Range, tasa As Double)
    '...
    For i = 1 To Caja.Rows.Count

...that Cash (and Caja) really needs to be a Range object, otherwise things can go all kinds of wrong.

Make these things explicit, pass parameters ByVal, and keep the same identifiers when concepts are the same:

Public Function TirNoPer360(ByVl Cash As Range, ByVal Dates As Range, ByVal rate As Double) As Double

Private Function Opt(ByVal Cash As Range, ByVal Dates As Range, ByVal rate As Double) As Double

Now it's much clearer that...

TirNoPer360 is the public UDF, and Opt is a private implementation detail of it.
Cash in TirNoPer360 is the same thing as what you had as Caja in Opt.
The function(s) return a Double

This block needs immediate attention:

If Opt(Cash, Dates, rate) > 0 And Opt(Cash, Dates, rate) > tolerance Then
    rate = rate + 0.0000001
ElseIf Opt(Cash, Dates, rate) < 0 And Opt(Cash, Dates, rate) < tolerance * -1 Then
    rate = rate - 0.0000001
Else
    rate = rate
End If

The entire Else block is no-op and should be removed. rate is already equal to rate, but the biggest inefficiency here is the fact that, in the worst case, Opt is invoked 4 times, and it's a very expensive function call. Pull it into a local variable (as was already suggested) and call it once:

Dim optResult As Double
optResult = Opt(Cash, Dates, rate)

And now the conditionals become:

If optResult > 0 And optResult > tolerance Then
    rate = rate + 0.0000001
ElseIf optResult < 0 And optResult < tolerance * -1 Then
    rate = rate - 0.0000001
End If

The tolerance looks like it should be an optional parameter, with a default value of 0.00001. You could amend the function's signature like this:

Public Function TirNoPer360(ByVl Cash As Range, ByVal Dates As Range, ByVal rate As Double, Optional ByVal tolerance As Double = 0.00001) As Double

Variable Iteration is not declared. I like that it's a fully spelled-out identifier and not just i, but given the variable has no use other than counting the number of iterations... so i wouldn't hurt at all. Just declare it - the fact that the code is allowed to run without this variable being declared indicates that Option Explicit is not specified at the top of the module, and that's a very dangerous thing, because any typo will merrily compile and run.. and produce the wrong results... and typos can be hard to spot. Spare yourself the trouble, and always put Option Explicit at the top of every module you ever write any code in.

  I need more than 100 tries in order to get a good rate

Your function needs to define a concept of what a "good rate" is, and bail out (Exit Function, or Exit For) when it gets one. Currently, the function performs 100 iterations every time, regardless of whether the 2nd or 99th iteration yielded a "good rate".

    TirNoPer360 = rate
Next Iteration

You're re-assigning the function's return value at every iteration. That's a redundant step, just assign it once, after the loop:

Next Iteration
TirNoPer360 = rate

Now, about that Opt function...

For i = 1 To Caja.Rows.Count
    sum = sum + Caja(i) / ((1 + tasa) ^ (Application.WorksheetFunction.Days360(Initial, Fecha(i)) / 360))
Next i

i should be declared as a Long, not an Integer. I don't know how many rows this function is iterating, but Caja being a Range means it's absolutely possible that there are more than 32,767 (the maximum value an Integer can take).

The function would be more efficient if you were iterating a single-dimensional Variant array rather than a Range.

Consider declaring Caja and Fecha parameters ByVal...As Variant, and passing them from TirNo360 like so:

Public Function TirNoPer360(ByVl Cash As Range, ByVal Dates As Range, ByVal rate As Double, Optional ByVal tolerance As Double = 0.00001) As Double

    Dim cashValues As Variant
    cashValues = Application.WorksheetFunction.Transpose(Cash.Value)

    Dim dateValues As Variant
    dateValues = Application.WorksheetFunction.Transpose(Dates.Value)

    Dim optResult As Double
    optResult = Opt(cashValues, dateValues, rate)

Now instead of this:

For i = 1 To Caja.Rows.Count
    sum = sum + Caja(i) / ((1 + tasa) ^ (Application.WorksheetFunction.Days360(Initial, Fecha(i)) / 360))
Next i

You'll have this (assuming the Caja parameter is now named cashValues):

Dim firstDate As Date
firstDate = dateValues(1)

For i = LBound(cashValues) To UBound(cashValues)
    sum = sum + cashValues(i) / ((1 + rate) ^ (Application.WorksheetFunction.Days360(firstDate, dateValues(i)) / 360))
Next

Now the only thing left to address, is the function names: I've no idea what TirNoPer360 actually stands for, and Opt is very cryptic. Give your functions meaningful names that ideally start with a verb, like, FindOptimalRate or something.


Merge Sort Implementation and Optimization This is Merge Sort that i implemented, i feel i can make it better esp the combination function.
So i split the code into functions because i don't like seeing a bunch of spaghetti code in a single function. I have the combination logic which combines the left and right array such lower elements go first.
def combine(left:list, right:list): #Combines both paths
    left.append(float('inf')) #So the Array is never empty
    right.append(float('inf')) #and the final element to compare is large

    combined_list = []
    while len(left) !=1 or len(right) != 1:
        if left[0] < right[0]:
            combined_list.append(left.pop(0))
        else:
            combined_list.append(right.pop(0))

    return combined_list

Next i have this recursive function which calls itself and combination logic then after reducing the array into singular pieces then returns the bigger sorted chunks and hence final array is hence sorted.
def merge_sort(array:list):
    half_length = len(array)//2
    left = array[:half_length]
    right = array[half_length:]

    if len(left) > 1:
        left = merge_sort(left)
    if len(right) > 1:
        right = merge_sort(right)

    return combine(left, right)

print(merge_sort([1,2,8,5,2,4,6,9,4,2]))

I feel that left.append(float('inf')) is a hack around, is there a better way of doing it spiting it into functions.

pop(0) has a linear time complexity, which makes the entire time complexity quadratic.

The single most important advantage of merge sort is stability: the elements compare equal retain their original order. In your code, if two elements are equal the one from the right is merged first, and stability is lost.


Helper function to make `setTimeout` synchronous I'm making a helper function to make setTimeout synchronous. Currently, it is asynchronous so other code runs before the timeout is done.

function syncSetTimeout(func, ms, callback) {
    (function sync(done) {
            if (!done) {
            setTimeout(function() {
                eval("(" + func + ")();");
                sync(true);
            }, ms);
            return;
        }
        eval("(" + callback + ")();");
    })();
}

func is a function that should be run after the specified timeout.
ms specifies how many milliseconds to wait before running func.
callback is the function that runs after the func is done running.

Is there a way to get rid of those eval statements? I've read that eval is evil. I'm also wondering if I can do this without a nested function. If there are any other ways to improve this code then please say so.

The Eval

Eval is indeed, the monster. There are plenty of reasons why you should not use it, although there are some situations where you are forced to use it. For sure, this is not the requirement in yours situation. If func is a function, you can call it using .apply.

function syncSetTimeout(func, ms, callback) {
    (function sync(done) {
        if (!done) {
            setTimeout(function() {
                func.apply(func);
                sync(true);
            }, ms);
            return;
        }
        callback.apply(callback);
    })();
}

Reference: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply

Or event without apply:

function syncSetTimeout(func, ms, callback) {
    (function sync(done) {
        if (!done) {
            setTimeout(function() {
                func();
                sync(true);
            }, ms);
            return;
        }
        callback();
    })();
}

The synchronous

You need to know, that trying to make setTimeout() sync means that you have some very bad design idea behind. You should not do it and you will not be able to make it really sync without killing performance of the browser and UX of your app.

I believe that you would like to create sync delay in your application. You are not the first who wants to do it (most of beginners or people with background from different language). You can take a look at answers to this question from stackoverflow: https://stackoverflow.com/questions/6921895/synchronous-delay-in-code-execution

From the answer to linked question (by OverZealous):

  JavaScript is a single-threaded language. You cannot combine
  setTimeout and synchronous processing. What will happen is, the timer
  will lapse, but then the JS engine will wait to process the results
  until the current script completes.

One reason people are trying to take advantage of sync delays is returning result from async code. This is not good strategy in JS. If you would like to return async results just use callbacks or more likely - Promises or Observables.


Listing curriculum vitae data I need this kind of ORDER BY to list my curriculum vitae data: when ep.data_termino is null it means that the job is your current job (you can have multiple current jobs), then I must list them first. Then, I need to list all the others ordered by the ep.data_termino ASC.

The IF was my only choice. I know that it is never a good option to use IFs in queries (at least that's how I learned it), so I am asking here if there is any better solution.

    SELECT ep.nome_empresa, ep.data_inicio, ep.descricao, ct.nome as contratacao_tipo_nome, p.nome as profissao_nome,

IF(ep.data_termino IS NULL,NOW(),ep.data_termino) as data_term

    FROM experiencia_profissional as ep
    JOIN contratacao_tipo as ct ON ct.codigo = ep.codigo_contratacao_tipo
    JOIN profissao as p ON p.codigo = ep.codigo_profissao
    JOIN experiencia_profissional_membro as epm ON epm.codigo_experiencia_profissional = ep.codigo
    JOIN membro as m ON m.codigo = epm.codigo_membro
    JOIN usuario as u ON u.codigo_membro = m.codigo

    WHERE u.codigo = 124
    ORDER by data_term DESC, ep.data_inicio ASC

Try:

ORDER BY ep.data_termino IS NOT NULL, ep.data_termino DESC, ep.data_inicio ASC

BTW,

IFNULL(ep.data_termino,NOW())

is a cleaner way to express:

IF(ep.data_termino IS NULL,NOW(),ep.data_termino)


Smart pointer that cannot be nullptr Sometimes I need a reference counting smart pointer that should not be nullptr. For example, I want to share a very heavy object without copying. And it's more convenient to place it on the heap. So I could use a shared_ptr. Also, I don't need to assign nullptr to this pointer or reassign it. So I don't want to check for nullptr in code that uses it. I think it would be useful to have a pointer that cannot be nullptr and cannot be reassigned.

I wrote a small prototype that has this features.

 #include <iostream>
 #include <string>
 #include <memory>
 #include <exception>
 #include <thread>

 using namespace std;

 // Box cannot hold nullptr
 template<class T>
 class Box
 {
 public:
     template <class... Args>
     static Box<T> create(Args&&... args)
     {
         return Box<T>(make_shared<T>(std::forward<Args>(args)...));
     }

     T * operator-> () const
     {
         return ptr.operator -> ();
     }
 private:
     Box(const shared_ptr<T> & ptr)
         : ptr(ptr)
     {}

     shared_ptr<T> ptr;
 };

 struct Test
 {
     Test(int i, double d, const string & s)
         : i(i), d(d), s(s)
     {
         cout << "Test ctor" << endl;
     }

     ~Test()
     {
         cout << "Test dtor" << endl;
     }

     Test(const Test & test) = delete;
     Test(Test && test) = delete;
     Test & operator = (const Test & test) = delete;
     Test & operator = (Test && test) = delete;

     int i;
     double d;
     string s;
 };

 void print(const Box<Test> & test)
 {
     // there is no need to check for nullptr
     cout << test->i << " " << test->d << " " << test->s << endl;
 }

 void twice(Box<Test> test)
 {
     // there is no need to check for nullptr
     test->i *= 2;
     test->d *= 2;
     test->s += test->s;
 }

 /*
 stdout:
 Test ctor
 42 3.14 hello
 84 6.28 hellohello
 Test dtor
 */
 int main()
 {
     Box<Test> test = Box<Test>::create(42, 3.14, "hello");
     print(test);

     thread t(twice, test);
     t.join();

     print(test);
 }

Is this code correct? And is this a good approach?

Update:
It looks like I found a ready solution https://github.com/dropbox/nn

Related discussions:

1) https://groups.google.com/a/isocpp.org/forum/#!topic/std-proposals/5TPClBA1fs8

2) http://boost.2283326.n4.nabble.com/Why-no-non-null-smart-pointers-td2642959.html

3) https://lists.boost.org/Archives/boost/2013/10/206732.php

Well, in principle having non-nullable pointers, smart or not, is a good thing. It's just a shame that nullability is not an optional non-default trait (not only for pointers, but for all types; for numbers non-zeroability might be an interesting companion-trait, which incidentally makes the former more efficient).

Your implementation of it leaves something to be desired though, because you completely block conversion from "normal" nullable pointers to your non-nullable one and back. That severely limits your types utility.
Also, one expects a lot more functionality from a pointer-like type, like more conversions, more operators, and the rest of the smart-pointer-interface, at least as far as compatible with the design-goals.

Now let's look at the code of your prototype:

Separate your new smart-pointer from the test-code, and make it a self-contained header-only-library.
Never use using namespace std; in an include-file, not that it is much better in a source-file. Read "Why is “using namespace std;” considered bad practice?" for the reasons.
As said above, you need loads more accessors, and all the other functons, member and free, which comprise the interface for a smart-pointer.
If you use private inheritance, you have an easier time passing through the needed interface with using than writing it from scratch.

And the code of your test:

Only explicitly flush a stream if you really need to. Doing so frivolously is quote costly.
If you want to output a single character to a stream, consider doing so explicitly instead of with a length-1 C-string. It's potentially slightly more efficient.


Java insertion sort implementation I have written code for insertion sort. I just want some feedback whether the above implementation is correct and can be improved.

public class InsertionSortNew {

    static int temp;

    public static void InsertionSort(int [] array){

        for(int i =0;i<=array.length-1;i++){

            int key = array[i];

            for(int j=i-1;j>=0;j--){

                if(key<array[j]){
                    temp = key;
                    array[j+1] = array[j];
                    array[j] = key;
                    key = array[j];

                }else{
                    break;
                }

            }
        }
    }

Code

First of all, please avoid having a static int temp: in case two or more threads call your insertion sort, they will interfere. Also, I would declare the constructor as private:

private InsertionSortNew() {}

Style

You abuse your code with empty lines. Usually people put empty lines before and after conditional blocks (if) and loops (for, while).

Performance

You can prune away like 60% of assignments in the inner loop as follows: first, you store the element being inserted; next, until sorted you keep moving the preceding larger integers one position to the right, and finally, you put the cached element into its correct position.

All in all, I had this in mind:

private InsertionSortNew() {}

public static final void sort(int [] array){
    for (int i = 1; i < array.length; ++i) {
        int key = array[i];
        int j;

        for (j = i - 1; j >= 0 && array[j] > key; --j) {
            array[j + 1] = array[j];
        }

        array[j + 1] = key;
    }
}

..., which gives me these performance figures:

Original version took 2281.41 milliseconds.
rodde's version took 1581.65 milliseconds.
Equals: true